[WIP] restructure rosidl to work with IDL files by dirk-thomas · Pull Request #298 · ros2/rosidl

dirk-thomas · 2018-09-19T23:11:23Z

These changes modify the rosidl processing pipeline to use .idl files (using a subset of OMG IDL 4.2). See https://github.com/ros2/ros2/blob/idl/README.rst#goals for a collection of bullets and references.

A high level overview of this work (the patch has been split up into separated PRs - see referenced PRs at the bottom of this ticket):

Currently this branch can be used to build some interface packages. For that all not yet changes message generators need to be skipped, e.g.:

Details

=== ./ament_cmake (git) ===
diff --git a/ament_cmake_core/cmake/core/ament_execute_extensions.cmake b/ament_cmake_core/cmake/core/ament_execute_extensions.cmake
index 9f29963..bee969e 100644
--- a/ament_cmake_core/cmake/core/ament_execute_extensions.cmake
+++ b/ament_cmake_core/cmake/core/ament_execute_extensions.cmake
@@ -23,6 +23,7 @@
 macro(ament_execute_extensions extension_point)
   if(AMENT_EXTENSIONS_${extension_point})
     foreach(_extension ${AMENT_EXTENSIONS_${extension_point}})
+     if(NOT "${extension_point}" STREQUAL "rosidl_generate_interfaces" OR "${_extension}" STREQUAL "rosidl_generator_c:rosidl_generator_c_generate_interfaces.cmake")
       string(REPLACE ":" ";" _extension_list "${_extension}")
       list(LENGTH _extension_list _length)
       if(NOT _length EQUAL 2)
@@ -36,6 +37,7 @@ macro(ament_execute_extensions extension_point)
       assert_file_exists("${_extension_file}"
         "ament_execute_extensions(${extension_point}) registered extension '${_extension_file}' does not exist")
       include("${_extension_file}")
+     endif()
     endforeach()
   endif()
 endmacro()

… type specific keyword

mikaelarguedas · 2018-09-19T23:34:41Z

I gave this a shot, it seems that it fails to build on Xenial / Python 3.5:

Traceback (most recent call last):
  File "/usr/lib/python3.5/runpy.py", line 184, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib/python3.5/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/mikael/work/ros2/idl_ws/build_debug/rosidl_adapter/rosidl_adapter/__main__.py", line 19, in <module>
    sys.exit(main())
  File "/home/mikael/work/ros2/idl_ws/build_debug/rosidl_adapter/rosidl_adapter/main.py", line 64, in main
    output_dir)
  File "/home/mikael/work/ros2/idl_ws/build_debug/rosidl_adapter/rosidl_adapter/__init__.py", line 42, in convert_to_idl
    output_dir / package_name / 'msg'))
  File "/home/mikael/work/ros2/idl_ws/build_debug/rosidl_adapter/rosidl_adapter/msg/__init__.py", line 35, in convert_msg_to_idl
    expand_template('msg.idl.em', data, output_file)
  File "/home/mikael/work/ros2/idl_ws/build_debug/rosidl_adapter/rosidl_adapter/resource/__init__.py", line 25, in expand_template
    if os.path.exists(output_file):
  File "/usr/lib/python3.5/genericpath.py", line 19, in exists
    os.stat(path)
TypeError: argument should be string, bytes or integer, not PosixPath

dirk-thomas · 2018-09-21T19:42:46Z

it seems that it fails to build on Xenial / Python 3.5:

I didn't try with Python 3.5 but it should be fixed by 9b0e9d0.

…tched rule

…an object representation

dirk-thomas · 2018-11-09T02:03:55Z

With the latest commit the pipeline works and the rosidl_generator_c has been updated to provide at least the same features as before. The changes in the test_interfaces.c file will provide a good idea how existing code will need to change:

ROS 1 msg/srv files using char will map to uint8 (as it is specified in ROS 1) rather then signed char as it was the case in the ROS 2 implementation
the rosidl_generator_c__<TYPE>__Array__* functions are using IDL type names rather then ROS 1 type names: bool -> boolean, byte -> octet, float32 -> float, float64 -> double

While this can't be merged until all message generators have been updated to process .idl files instead of .msg / .srv files I am looking for some early feedback especially on how the message generator itself and its templates are organized / changed:

This new structure avoids any repetition in the templates (e.g. between messages and services) and allows easy integration of action and other interface definitions in the future. Also generators unable to handle new types like actions would automatically gracefully fall back to do nothing.

sloretz

Partial review. The changes in test_interfaces.c looks good to me. I didn't look as closely as I would like at rosidl_generator_c/__init__.py, but the templates themselves look fine.

sloretz · 2018-09-21T17:30:14Z

rosidl_adapter/rosidl_adapter/__main__.py

+
+from rosidl_adapter.main import main
+
+sys.exit(main())


Does this need to be an entry point to work on windows?

It doesn't since it is invoked with python -m rosidl_adapter.

rosidl_adapter/rosidl_adapter/resource/srv.idl.em

rosidl_cmake/cmake/rosidl_generate_interfaces.cmake

sloretz · 2018-11-09T18:52:49Z

rosidl_generator_c/rosidl_generator_c/__init__.py

+                locator, png_file=os.path.join(
+                    args['output_dir'], str(idl_rel_path.parent),
+                    idl_rel_path.stem) + '.png')
+            for template_file, generated_filename in mapping.items():


Can multiple messages or service definitions live in the same IDL file? If so, can a generator generate code for messages and services in an IDL file while ignoring an IDL module or struct describing an action?

I'm wondering if it would be possible to add a step to the pipeline where an <action>.idl can be converted to an <action_plus_generated_messages_and_services>.idl. Some generators (including rmw specific ones) parse the idl, find messages and services, and generate code for messages and services. Other generators (not rmw specific ones) get the same file, parse it, find an action, and generate code for only the action.

Nevermind, I see now that parse_idl_string() allows one message or one service per file.

Instead, would it be possible to add a step after rosidl_adapter but before calling the generators that parses an <action>.idl and creates a <send goal service>.idl, <get result service>.idl and <feedback message>.idl?

Can multiple messages or service definitions live in the same IDL file? If so, can a generator generate code for messages and services in an IDL file while ignoring an IDL module or struct describing an action?

Nevermind, I see now that parse_idl_string() allows one message or one service per file.

Yeah, to keep the complexity down it only accepts single definitions in the parse functions. The actual logic in the C generator though is capable of handling any number of definitions. So the parse functions can certainly be extended in the future to enable that.

I'm wondering if it would be possible to add a step to the pipeline where an <action>.idl can be converted to an <action_plus_generated_messages_and_services>.idl.

Instead, would it be possible to add a step after rosidl_adapter but before calling the generators that parses an <action>.idl and creates a <send goal service>.idl, <get result service>.idl and <feedback message>.idl?

I don't think these files need to be generated explicitly. When an .action file is parsed (actually the equivalent .idl file) the object representation can provide these extended derived definitions. So each generator handling an action can feed these sub-parts to an existing message / service template. Similar to how a service objects contains two message objects and just invoked the message template twice - once for the request, once for the response - and optionally generates something service specific in addition. The action object would just contain not only plain messages / services which have 1-to-1 the same content as in the .action file but contain the "enriched" versions of the derived types.

sloretz · 2018-11-09T20:00:13Z

rosidl_generator_c/resource/idl__functions.c.em

+@{
+from rosidl_parser.definition import Service
+}@
+@[for service in content.get_elements_of_type(Service)]@


IIUC parse_idl_string() will only result in an IdlContent created with one service definition. Is this a hook for supporting multiple services in an IDL file in the future?

Same as #298 (comment) In the future parse_idl_string should be extended to support multiple definitions in a single file. It is just more complex to do so I haven't done it in this round.

dirk-thomas · 2019-01-25T21:01:46Z

Closing in favor of #346 and #334.

dirk-thomas added 6 commits September 19, 2018 10:12

copy ROS msg/srv parser

cf56be9

remove message/service specific stuff from the other module

48635bc

add rosidl_adapter package

b1caf0d

use rosidl_parser in rosidl_cmake, allow passing interface files with…

bd944f8

… type specific keyword

add lark grammar and parsing functions

552b08c

update rosidl_generator_c to use IDL files

9149d16

dirk-thomas added enhancement New feature or request in progress Actively being worked on (Kanban column) labels Sep 19, 2018

dirk-thomas self-assigned this Sep 19, 2018

dirk-thomas mentioned this pull request Sep 19, 2018

ROS 2 Crystal Clemmys ros2/ros2#529

Closed

34 tasks

fix Python 3.5 issue

9b0e9d0

dirk-thomas and others added 17 commits September 21, 2018 13:02

only require either messages or services

43a9052

add u16string type and functions and use for wstring, add long double

2425af5

fix for 43a9052

166d457

[rosidl_generator_c] fix (w)string with upper bound

e177d0b

get_filename_component(... EXT) includes '.'

66f9129

Try to convert message and service files

34b5c46

Further work on IDL.

4393367

Include message name in constants module

0a4af16

1.125 is a legal floating point literal

028b117

multiple annotation_appl per member

75593a4

add string literal escaping

b5df207

First attempt at char literal escaping

0bee962

More informative error message.

88569ee

Change char mapping to int8.

94dbeeb

String escaping fixup.

9cc7654

Check escape before graphic char

a069cbc

.srv use string literal escape function

7aa8d9d

dirk-thomas added 6 commits October 19, 2018 15:36

add additional rule to grammar to distinguish relevants parts of a ma…

0aee9b7

…tched rule

add priorities / weights to prefer certain rules

499028e

add new object representation for interfaces

25b4f1e

add parser to parse .idl files using lark and transform the AST into …

5fdbd8c

…an object representation

add test for .idl parser

bc9578f

working copy dump

a9fd325

dirk-thomas mentioned this pull request Nov 9, 2018

heuristic to extract comments from .msg files #316

Closed

extract comments and units

4635a93

sloretz reviewed Nov 9, 2018

View reviewed changes

dirk-thomas added 4 commits November 9, 2018 13:35

replace function pointer with import

6bd82b0

rm obsolete file

49463a6

migrate rosidl_generator_cpp, fix rosidl_generator_c

9116396

avoid duplicate logic

5ee3cb0

apojomovsky mentioned this pull request Nov 26, 2018

Change UUID type in action msgs #332

Merged

dirk-thomas mentioned this pull request Jan 25, 2019

Migrate to IDL #346

Closed

8 tasks

dirk-thomas closed this Jan 25, 2019

dirk-thomas deleted the rosidl-reloaded branch January 25, 2019 21:01

dirk-thomas removed the in progress Actively being worked on (Kanban column) label Jan 25, 2019

dirk-thomas mentioned this pull request Feb 21, 2019

change generators to IDL-based pipeline ros2/rosidl_typesupport#39

Merged

sloretz mentioned this pull request Jan 13, 2021

Add generalized interface generation architecture proposal ros2/design#310

Open

Conversation

dirk-thomas commented Sep 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mikaelarguedas commented Sep 19, 2018

Uh oh!

dirk-thomas commented Sep 21, 2018

Uh oh!

dirk-thomas commented Nov 9, 2018

Uh oh!

sloretz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dirk-thomas commented Jan 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dirk-thomas commented Sep 19, 2018 •

edited

Loading