Allow test_communication to use idl messages #549
Conversation
Signed-off-by: Miguel Company <[email protected]>
| @@ -41,8 +41,8 @@ if(BUILD_TESTING) | |||
| foreach(interface_file ${interface_files}) | |||
| get_filename_component(interface_ns "${interface_file}" DIRECTORY) | |||
| get_filename_component(interface_ns "${interface_ns}" NAME) | |||
| string_ends_with("${interface_file}" ".msg" is_message) | |||
| if(is_message AND interface_ns STREQUAL "msg") | |||
| string_ends_with("${interface_file}" ".idl" is_idl) | |||
There was a problem hiding this comment.
I see current change works with test_communication pacakge, but more generally what we can do is,
- if
.idl, we can categorize them based on namespace. - and other files
.msg,.srvand.actioncan be categorized with file extensions and namespace.
what do you think?
There was a problem hiding this comment.
The official definition language is currently idl.
When .msg, .srv and .action files are processed, they are converted into .idl, and both files are installed and returned by ament_index_get_resource(interface_files "rosidl_interfaces" "<pkg_name>")
For instance, the contents of the file in install/share/ament_index/resource_index/rosidl_interfaces/test_msgs are the following:
action/Fibonacci.action
action/Fibonacci.idl
action/NestedMessage.action
action/NestedMessage.idl
msg/Arrays.idl
msg/Arrays.msg
msg/BasicTypes.idl
msg/BasicTypes.msg
msg/BoundedPlainSequences.idl
msg/BoundedPlainSequences.msg
msg/BoundedSequences.idl
msg/BoundedSequences.msg
msg/Builtins.idl
msg/Builtins.msg
msg/Constants.idl
msg/Constants.msg
msg/Defaults.idl
msg/Defaults.msg
msg/Empty.idl
msg/Empty.msg
msg/MultiNested.idl
msg/MultiNested.msg
msg/Nested.idl
msg/Nested.msg
msg/Strings.idl
msg/Strings.msg
msg/UnboundedSequences.idl
msg/UnboundedSequences.msg
msg/WStrings.idl
msg/WStrings.msg
srv/Arrays.idl
srv/Arrays.srv
srv/BasicTypes.idl
srv/BasicTypes.srv
srv/Empty.idl
srv/Empty.srv
So we should filter the list to remove duplicates. I think we should stick to .idl files.
We could take this a step further, and have a specific CMake function in either ament_index or rosidl_cmake to return the list of .idl in the ament index.
There was a problem hiding this comment.
I see. In that case, to keep the consistent check for all endpoints, we also want to change the line 49 and 50 for service files to use is_idl and namespace check?
Signed-off-by: Miguel Company <[email protected]>
Signed-off-by: Miguel Company <[email protected]>
|
@methylDragon @clalancette can you review this? |
|
@clalancette can you assign this to yourself? (from waffle triage, but i do not have power...) |
|
@clalancette Friendly ping |
Opening this PR as requested on #542 (comment).
The CMake on
test_communicationpackage assumes that the messages intest_msgspackage are defined as.msgfiles.Since
.msgfiles are converted into.idlfiles, and both are installed as resources in the ament index, we can always use the.idlones, thus allowing messages to be defined directly in.idlfiles.