DDSRPC_ — Review comments Remedy IT on RPC4DDS

This is not a 100% review, we focused on the basic profile together with the IDL and after receiving the initial comments of the AB and the fact that there is not one submission we didn't complete a detailed review. We didn't review the github code.

IDL zip (mars/14-11-05)
rpc_types.idl:
Uses the RTI specific top-level annotation. Also top-level is illegal according to the IDL grammar.
Uses anonymous types
robot.idl:

Document (mars/14-11-02)
Section 4
reference to CORBA should be updated to CORBA v3.3
reference to IDL should be updated to CORBA v3.3
reference for DDS4CCM lacks document number
reference for DDS-Cxx-PSM and DDS-Java-PSM should be updated to the formal version
reference to github, how can we in the future pinpoint the exact version of the files mentioned, shouldn't the reference be more explicit about which version?
reference to IDL 3.5 is lacking (used in section 8.3.1)

Section 5
Shouldn't there be a reference in section 4 for this quote?

Section 8.1
The spec talks about “communication patterns”, but in the context of UCM we are talking about “interaction patterns”, what about adopting the UCM term?

Section 8.2.1
In the second paragraph its says “the service implementation contains a data reader to read the method name”, but shouldn't it be “the service implementation contains a data reader which receives the request and after that reads the method name and the parameters.”
Can't the diagram be placed after the first paragraph, it is now on the next page but we refer in text already to it).
Third paragraph, at the end “are filtered out”.
Second paragraph, the fact that a content-based filter “is” used forces that everyone must do this (which is the most logical implementation), but shouldn't it be “could”
In the 4rd paragraph it mentions “asynchronous invocations”, but this is not mentioned in 8.2.2.1, how do asynchronous invocations look like with the function call style in the basic profile?

Section 8.2.3
Isn't it better to keep the term “client” instead of introducing “requester”
Where in this document is the API defined to get the request id at both sides for implicit or explicit?

Section 8.2.5
“ it is” instead of “It is”

Section 8.3.1
What about the support for modules in IDL. In larger systems it will almost get impossible to guarantee the uniqueness of an interface when multiple parts are integrated. Also with an UCM approach modules are the way to group interfaces together. The module is now completely left which could lead to problems in a larger system. To our idea a change of an interface to another module is something that RPC4DDS shouldn't silently adapt to

Section 8.3.1.2
Use a reference to DDS-XTypes in the first paragraph, not in the 3rd
Section xx
What about collisions in the implied IDL types, hashes, etc. If a collision appears what are the rules to follow, because client and service could be developed by independent groups and DDS vendors which should than follow the same rules

Section xx
The spec uses IDL anonymous types in some places, these are deprecated and prevented to our idea.

Section 8.4
“by the used,”??

Section 8.2.1
Why include space as part of user_def_alpha_num?

Section 8.4.2.3
Reference error

Section 8.4.3.3
Reference error

Section 8.5.1.1.1
Why is SequenceNumber_t not using an “unsigned long long”. It is now a struct but there is no explanation in the specification about high and low

Section 8.5.1.1.2
Space missing before “shall” on 3rd line
Why not put the implied types into a nested module so that they don't pollute the user IDL module and resulting code, doxygen documentation, etc

Section 8.5.1.1.3
For the result unions, why is not the unknownEx be part of the RobotControl_Return union as described on page 25, that would save a lot of union members. This exception is generic and independent of the operation.
What when the user has an interface with a user defined parameter with the name return_ and there is a return value? This leads to a clash in the generated struct
For the result struct, with IDL attributes it is possible to define separate exceptions with the set and the get of the attribute (getraises/setraises), this is not reflected in the result mapping
What if there is a clash between the const hash name and user defined IDL?

Section 8.5.1.1.4
Why not put the implied types into a nested module so that they don't pollute the user IDL module and resulting code, doxygen documentation, etc
Typo Non-noramtive

Section 8.5.1.2.1
Shouldn't the dds::rpc scope be used when the annotations are used?
We still doubt the need to introduce @choice, with a union with the hash as discriminator value the is always one value active and there is a quick lookup at runtime. It works for the basic profile so why introduce this new IDL behavior for the enhanced one?
What is the behavior of @autoid with a hash-collision? Also what happens when operations are reordered at the moment there is a hash-collision? Couldn't that break interoperability?
What when the user has an operation remoteEx in his interface IDL?

Section 8.5.2
What is the mapping for the return codes when the regular IDL to IDL2C+, IDL2C+11, etc language mappings are used
What is the error when the remote service is not there (for example not running or not reachable), which exception do I get back?
What when the service crashes during handling of the request, what kind of error do I get back, what is the timeout? How can configure timeouts etc?

Section 8.7.1
Evolution should be evolution.

Section 8.7.1.3
What about changing the exception specification of an operation?

Section 8.8.2
Not a legal IDL struct definition, struct has to be removed from its members

Section 8.8.2.1/8.8.2.2
Add a reference to where in this specification this language binding is provided

Section 8.10.1
What about the library name RTI DDS currently supports. It is very helpful to have multiple QoS profiles in one file and the spec should define how this works

Split issue in multiple smaller issues

Split issue in multiple smaller issues.