Python typesupport for Actions #21

apojomovsky · 2018-11-15T16:21:47Z

Closes #14
This PR:

Adds the required rosidl_python/rosidl_generator_py/resource/_action.py.em template file with the proper imports for the generated action messages/services.
Adds action typesupport bits to the _msg_pkg_typesupport_entry_point.c.em script.
Adds logic to the rosidl_generator_py so that it can handle the new action's template.

brawner · 2018-12-21T02:46:38Z

The fork this PR is coming from is not accessible. Was it dropped, or just not made public?

apojomovsky · 2018-12-21T15:18:54Z

Hi @brawner , I think the branch name has lead to confusion here. Actually, this PR is coming from a local branch to the rcl repo and can be found here: https://github.com/ros2/rosidl_python/tree/apojomovsky/typesupport_for_actions

brawner · 2018-12-21T16:19:57Z

Haha, that indeed was my confusion. Thanks!

…

On Dec 21, 2018, at 7:18 AM, Alexis Pojomovsky ***@***.***> wrote: Hi @brawner , I think the branch name has lead to confusion here. Actually, this PR is coming from a local branch to the rcl repo and can be found here: https://github.com/ros2/rosidl_python/tree/apojomovsky/typesupport_for_actions — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

brawner · 2018-12-27T23:17:07Z

I've tested this out on my system. Is there a reason why no high-level action class is created? Something like:

from test_msgs.action import Fibonacci

as opposed to

from test_msgs.action import Fibonacci_Goal, Fibonacci_Result, Fibonacci_Feedback

jacobperron · 2019-01-07T22:55:37Z

I've tested this out on my system. Is there a reason why no high-level action class is created? Something like:
 from test_msgs.action import Fibonacci

It would appear that the _action.py.em template is not being invoked. It's possibly because of current rosidl pipeline where (I think) .action files are being broken into services/messages prior to being passed to the generators. @apojomovsky Can you confirm?

apojomovsky · 2019-01-08T15:16:56Z

Hi @jacobperron , I've just verified and it's exactly as you say. With the current pipeline, action files are broken into services and messages prior to being passed to the generator.

dirk-thomas · 2019-01-09T21:06:38Z

rosidl_generator_py/cmake/custom_command.cmake

@@ -23,7 +23,8 @@ add_custom_command(
  COMMAND ${PYTHON_EXECUTABLE} ${rosidl_generator_py_BIN}
  --generator-arguments-file "${generator_arguments_file}"
  --typesupport-impls "${_typesupport_impls}"
-  DEPENDS ${target_dependencies} ${rosidl_generate_interfaces_TARGET}
+  ${extra_generator_arguments}


Where is this variable being defined?

A left over from a previous attempt. Thanks for pointing it out. Removed.

hidmic · 2019-01-10T16:15:26Z

Alright, it seems to be working now. Or at least I'm able to import the Fibonacci class from test_msgs.action and get its C type support with Fibonacci.__import_type_support__().

Having said that, I'm not very proud of this patch. It works, but it's fragile, if you know what I mean. The split pipeline does no good to the Python generator implementation, and this is the solution I came up with that minimized the amount of changes necessary. Hopefully, we'll get to replace it with a more robust implementation once Dirk's work lands.

@jacobperron this is ready for you to implement actions in rclpy (and catch bugs, if any, that's the ultimate test).

hidmic · 2019-01-10T16:20:19Z

Running CI on this one in the meantime:

Linux
Linux-aarch64
macOS
Windows

sloretz · 2019-01-10T18:43:53Z

rosidl_generator_py/resource/_action.py.em

+print('%sif %s.Metaclass._TYPE_SUPPORT is None:' % (' ' * 4 * 3, '_cancel_goal'))
+print('%s%s.Metaclass.__import_type_support__()' % (' ' * 4 * 4, '_cancel_goal'))
+}@
+


Need an extra blank line to have two between top-level definitions

True. Fixed.

sloretz · 2019-01-10T21:15:27Z

rosidl_generator_py/rosidl_generator_py/generate_py_impl.py

+            content = re.sub(
+                r"# BEGIN %s$.*^# END %s" % (block_name, block_name),
+                '', content, 0, re.M | re.S
+            )


Is this part where it modifies the previously generated __init__.py the fragile part you were referring to?

That and adding _action to the package name whenever necessary (but not everywhere) to avoid name collisions.

properly

sloretz

LGTM with green CI. I think this could be merged now, but shouldn't be released until rclpy actions are ready in case there are bugs not caught by review.

hidmic · 2019-01-11T16:29:15Z

@sloretz fully agree.

hidmic · 2019-01-11T20:57:36Z

If this thread is right, 3e212cb should fix our little issue.

hidmic · 2019-01-14T12:57:43Z

Re-running Windows CI, last one failed for unrelated reasons:

20:56:50 II> Using workspace: ws
20:56:50 WW> Deleting the folder at 'ws'.
20:56:50 Traceback (most recent call last):
20:56:50   File "C:\Python37\lib\shutil.py", line 389, in _rmtree_unsafe
20:56:50     os.unlink(fullname)
20:56:50 PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'ws\\log\\test_2019-01-10_22-14-13\\logger_all.log'
20:56:50 
20:56:50 During handling of the above exception, another exception occurred:
20:56:50 
20:56:50 Traceback (most recent call last):
20:56:50   File "run_ros2_batch.py", line 32, in <module>
20:56:50     sys.exit(main())
20:56:50   File "C:\J\workspace\ci_windows\ros2_batch_job\__main__.py", line 132, in main
20:56:50     return run(args, build_function, blacklisted_package_names=blacklisted_package_names)
20:56:50   File "C:\J\workspace\ci_windows\ros2_batch_job\__main__.py", line 404, in run
20:56:50     remove_folder(args.workspace)
20:56:50   File "C:\J\workspace\ci_windows\ros2_batch_job\util.py", line 114, in remove_folder
20:56:50     shutil.rmtree(path, onerror=del_rw)
20:56:50   File "C:\Python37\lib\shutil.py", line 507, in rmtree
20:56:50     return _rmtree_unsafe(path, onerror)
20:56:50   File "C:\Python37\lib\shutil.py", line 386, in _rmtree_unsafe
20:56:50     _rmtree_unsafe(fullname, onerror)
20:56:50   File "C:\Python37\lib\shutil.py", line 386, in _rmtree_unsafe
20:56:50     _rmtree_unsafe(fullname, onerror)
20:56:50   File "C:\Python37\lib\shutil.py", line 391, in _rmtree_unsafe
20:56:50     onerror(os.unlink, fullname, sys.exc_info())
20:56:50   File "C:\J\workspace\ci_windows\ros2_batch_job\util.py", line 113, in del_rw
20:56:50     os.remove(name)
20:56:50 PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'ws\\log\\test_2019-01-10_22-14-13\\logger_all.log'

hidmic · 2019-01-14T18:33:29Z

Alright!! All green. Going in.

dirk-thomas · 2019-01-29T18:56:32Z

rosidl_generator_py/resource/_action.py.em

+        else:
+            cls._TYPE_SUPPORT = module.type_support_action__@(subfolder)_@(module_name)
+@{
+preffix = '_' + convert_camel_case_to_lower_case_underscore(spec.action_name) + '__'


Spelling: prefix.

Fixed in #28

* Adds Python typesupport for Actions (#21) * Add _action.py.em; Makes rosidl_generator_py handle action files properly * Adds CMake pipeline bits for action generation. * Fixes all linter issues. * Attempts to fix CMake warnings in Windows. * Fix flake8 error (#27) * Provide type support for 'goal_status_array' in action type support Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Fix spelling typo Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Ignore import order on generated imports (#29) Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

apojomovsky requested review from sloretz and hidmic November 15, 2018 16:21

tfoote added the in progress Actively being worked on (Kanban column) label Nov 15, 2018

This was referenced Dec 28, 2018

Adding logic for actions ros-visualization/rqt#186

Merged

Adding logic for rqt_action ros-visualization/rqt_msg#4

Merged

Porting rqt_action to ROS2 ros-visualization/rqt_action#4

Merged

dirk-thomas reviewed Jan 9, 2019

View reviewed changes

hidmic force-pushed the apojomovsky/typesupport_for_actions branch from cb12148 to 615e6a2 Compare January 10, 2019 16:05

hidmic added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jan 10, 2019

hidmic removed their request for review January 10, 2019 16:21

sloretz reviewed Jan 10, 2019

View reviewed changes

Alexis Pojomovsky and others added 3 commits January 11, 2019 12:56

Add _action.py.em; Makes rosidl_generator_py handle action files

2770a78

properly

Adds CMake pipeline bits for action generation.

234ce1a

Fixes all linter issues.

161faf0

hidmic force-pushed the apojomovsky/typesupport_for_actions branch from 2e51b21 to 161faf0 Compare January 11, 2019 15:58

sloretz approved these changes Jan 11, 2019

View reviewed changes

Attempts to fix CMake warnings in Windows.

3e212cb

hidmic merged commit 29848db into master Jan 14, 2019

tfoote removed the in review Waiting for review (Kanban column) label Jan 14, 2019

hidmic deleted the apojomovsky/typesupport_for_actions branch January 14, 2019 18:45

dirk-thomas reviewed Jan 29, 2019

View reviewed changes

jacobperron mentioned this pull request Mar 7, 2019

[Crystal patch 3 backport] Ignore import order on generated imports (#29) #31

Closed

nuclearsandwich mentioned this pull request Mar 8, 2019

Crystal Patch Release 3 ros2/ros2#656

Closed

10 tasks

dirk-thomas mentioned this pull request Apr 15, 2019

remove obsolete argument mod_prefix #41

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python typesupport for Actions #21

Python typesupport for Actions #21

apojomovsky commented Nov 15, 2018

brawner commented Dec 21, 2018

apojomovsky commented Dec 21, 2018

brawner commented Dec 21, 2018 via email

brawner commented Dec 27, 2018

jacobperron commented Jan 7, 2019 •

edited

Loading

apojomovsky commented Jan 8, 2019

dirk-thomas Jan 9, 2019

hidmic Jan 10, 2019

hidmic commented Jan 10, 2019

hidmic commented Jan 10, 2019 •

edited

Loading

sloretz Jan 10, 2019

hidmic Jan 11, 2019

sloretz Jan 10, 2019

hidmic Jan 11, 2019

sloretz left a comment

hidmic commented Jan 11, 2019

hidmic commented Jan 11, 2019

hidmic commented Jan 14, 2019 •

edited

Loading

hidmic commented Jan 14, 2019

dirk-thomas Jan 29, 2019

jacobperron Jan 29, 2019

Python typesupport for Actions #21

Python typesupport for Actions #21

Conversation

apojomovsky commented Nov 15, 2018

brawner commented Dec 21, 2018

apojomovsky commented Dec 21, 2018

brawner commented Dec 21, 2018 via email

brawner commented Dec 27, 2018

jacobperron commented Jan 7, 2019 • edited Loading

apojomovsky commented Jan 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hidmic commented Jan 10, 2019

hidmic commented Jan 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sloretz left a comment

Choose a reason for hiding this comment

hidmic commented Jan 11, 2019

hidmic commented Jan 11, 2019

hidmic commented Jan 14, 2019 • edited Loading

hidmic commented Jan 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobperron commented Jan 7, 2019 •

edited

Loading

hidmic commented Jan 10, 2019 •

edited

Loading

hidmic commented Jan 14, 2019 •

edited

Loading