store types as tuple and return ordered dict

[mikaelarguedas]

This is a follow-up of #19
Required to fix ros2/ros2cli#59

@dirk-thomas the returned keys right now are the exact names of the slots (with leading underscore), is it what you expected in your review of #19 (comment) or should the leading underscore be stripped ?

[dirk-thomas]

Beside two minor nitpicks I think it would make more sense to store an actual type object in the new class variable SLOT_TYPES - as suggested in the referenced comment:

Change the data structure from a mutable dict to an immutable tuple of abstract types

E.g. instead of storing the string rosidl_generator_py/Primitives[<=3] which the caller has to parse I suggested to store rosidl_parser.definition.BoundedSequence(rosidl_parser.definition.NamespacedType(['rosidl_generator_py', 'msg'], 'Primitives'), 3) (verbose to clarify which types I was referring to).

[dirk-thomas]

Why not import the module at the top of the file?

[dirk-thomas]

Why two parenthesis here? One should be sufficient.

[mikaelarguedas]

I think it would make more sense to store an actual type object in the new class variable SLOT_TYPES - as suggested in the referenced comment:

Thanks for the clarification, I'll update to use these types instead.

[mikaelarguedas]

@dirk-thomas Looking a bit more into this, it looks like this work will be conflicting / duplicating effort with #24.

Would it be better to wait for #24 to land and build based on the state off master at that point?

---

If not, there is an initial working version at mikaelarguedas@0a88958 that I can push forward. A few questions remain:
How would consumer parse these types?
One option is to provide helper functions in a rosidl package to:

• know if a basetype is a ROS primitive type or not (e.g. string is a ROS basic type but String is not a rosidl BasicType)
• import a ROS module based on the AbstractType if it;s a complex type

Based on this the consumer could instantiate messages of that type and use them (use case from ros2/ros2cli#197)

[dirk-thomas]

Looking a bit more into this, it looks like this work will be conflicting / duplicating effort with #24.

That will certainly be the case.

Would it be better to wait for #24 to land and build based on the state off master at that point?

That would be great (to avoid the need for rebasing the patch). I hope that the referenced PR will get merged in the near future.

How would consumer parse these types?

I am not sure I understand what you mean with "parse these types"? Since the API would return types (rather than strings) there shouldn't be a need to parse anything.

• know if a basetype is a ROS primitive type or not (e.g. string is a ROS basic type but String is not a rosidl BasicType)

I don't think that distinction is relevant. At least in all the message generators patched for the IDL work I don't recall anywhere where a condition would say "isinstance(t, BaseType) or isinstance(t, BaseString)".

import a ROS module based on the AbstractType if it;s a complex type

A function to import the module if the type is a NamespaceType might indeed be helpful.

[mikaelarguedas]

Superseeded by #33