Adding a get_slot_fields_and_types method to python msg classes by mlautman · Pull Request #19 · ros2/rosidl_python

mlautman · 2018-11-03T01:53:28Z

This adds a _slot_types field to python message classes as was requested here: #16

This attempts to remain as true to the ROS1 functionality as possible

mlautman · 2018-11-03T01:53:45Z

dirk-thomas · 2018-11-05T21:11:51Z

rosidl_generator_py/resource/_msg.py.em

+@[for field in spec.fields]@
+@[  if field.type.is_primitive_type() ]@
+@[    if field.type.is_array ]@
+        '@(get_python_type(field.type))[@(field.type.array_size)]',


This doesn't distinguish between a static array (with a fixed size) and a dynamic array (either with an upper bound or without a bound).

True. Do you have a preferred method to denote the different types of arrays? I didn't have a strong intuition here

I would suggest either the same notation as in the .msg or an object representation. Thinking about it the second one is probably be better in terms of stability with the upcoming change to .idl files since the notation will change (see https://github.com/ros2/design/blob/gh-pages/articles/142_idl.md).

Looking at the patch I don't see where the type is being distinguished?

My apologies, I got my branches mixed up. All of my changes are in one commit (9454002) but I'm unsure of whether I should cherry pick that commit on top of master or 0.5.2. Since this is targeting master, I assume that I should cherry pick on top of that, but doing so causes the build to fail for unrelated reasons..

/home/mike/ws_rqt2/build/rosidl_generator_py/rosidl_generator_py/rosidl_generator_py/msg/_string_arrays_s.c:12:10: fatal error: rosidl_generator_c/primitives_sequence.h: No such file or directory #include <rosidl_generator_c/primitives_sequence.h> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. /home/mike/ws_rqt2/build/rosidl_generator_py/rosidl_generator_py/rosidl_generator_py/msg/_nested_s.c:12:10: fatal error: rosidl_generator_c/primitives_sequence.h: No such file or directory #include <rosidl_generator_c/primitives_sequence.h> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. /home/mike/ws_rqt2/build/rosidl_generator_py/rosidl_generator_py/rosidl_generator_py/msg/_various_s.c:12:10: fatal error: rosidl_generator_c/primitives_sequence.h: No such file or directory #include <rosidl_generator_c/primitives_sequence.h>

Is there a .repos available with the most recent ros2 branches that I should use for making PR's to master?

@dirk-thomas

It depends on what you are trying to build. I would suggest using the .repos files from the master branch in the ros2/ros2 repo and building everything from source for your use case. Then a patch based on the master of this repo will work.

(The opposite would be to use the bouncy branch of the .repos file and then also target the bouncy branch in this repo. But usually all contributions are targeting the default branch and only in case of severe bug fixes are being considered for backporting afterwards.)

The current state seems to pass on CI:

mlautman · 2018-11-07T02:40:03Z

@dirk-thomas ready for another review

rosidl_generator_py/resource/_msg.py.em

mlautman · 2018-11-16T17:45:15Z

@dirk-thomas Changed _slot_types_dict to get_slot_types_dict() and added noqa W504 where appropriate to pass flake8

mlautman · 2018-11-16T18:52:47Z

https://ci.ros2.org/job/ci_linux/5564/

dirk-thomas · 2018-11-17T02:11:51Z

Ignoring W504 all over the place isn't necessary. This has been addressed in the linter configuration already a while ago: ament/ament_lint#110.

mlautman · 2018-11-19T17:37:17Z

@dirk-thomas It still fails locally with colcon build test --package-select rosidl_python without that change

dirk-thomas · 2018-11-19T17:49:53Z

It still fails locally

Are you using the latest default branches of all the other ROS 2 repos?

mlautman · 2018-11-19T18:55:03Z

rosidl_generator_py/resource/_msg.py.em

        return True
+
+    @@classmethod
+    def get_slot_types_dict(cls):


Suggested change

def get_slot_types_dict(cls):

def get_slot_fields_and_types(cls):

rosidl_generator_py/test/test_interfaces.py

rosidl_generator_py/resource/_msg.py.em

brawner · 2018-11-19T19:04:36Z

rosidl_generator_py/resource/_msg.py.em

+
+    @@classmethod
+    def get_fields_and_field_types(cls):
+        return cls._slot_types_dict


Since this is a getter, should it return a shallow copy so that you can modify the underlying dictionary?

Good catch. Fixed.

mlautman · 2018-11-19T20:50:18Z

@dirk-thomas I am using the ros2.repos that you passed along to us with the 0.5.2 release.

I went back and removed W504. I also changed the code to return a copy of the dict rather than the dict itself per @brawner's comment

rosidl_generator_py/resource/_msg.py.em

dirk-thomas · 2018-11-19T22:06:32Z

This can be merged as is for now. With the coming change I will likely change this in the following way:

Store the types in a class constant, e.g. SLOT_TYPES.
Change the data structure from a mutable dict to an immutable tuple of abstract types
Either remove the class method (or rename it to get_slot_types which zips __slots__ and SLOT_TYPES and returns an ordered dictionary).

tfoote added the in review Waiting for review (Kanban column) label Nov 3, 2018

mlautman mentioned this pull request Nov 3, 2018

rqt_py_common port ros-visualization/rqt#148

Merged

dirk-thomas reviewed Nov 5, 2018

View reviewed changes

mlautman force-pushed the adding-slot-types-to-python-msgs branch 2 times, most recently from 7ed6394 to 47cf04c Compare November 5, 2018 22:15

clalancette requested a review from dirk-thomas November 8, 2018 18:10

mlautman mentioned this pull request Nov 9, 2018

Adjusting message_tree_model for ros2 ros-visualization/rqt#156

Closed

mlautman force-pushed the adding-slot-types-to-python-msgs branch from 47cf04c to 9454002 Compare November 13, 2018 21:47

brawner mentioned this pull request Nov 14, 2018

Get public property names through method or public property (i.e. public __slots__) #20

Closed

dirk-thomas reviewed Nov 15, 2018

View reviewed changes

rosidl_generator_py/resource/_msg.py.em Outdated Show resolved Hide resolved

mlautman force-pushed the adding-slot-types-to-python-msgs branch 2 times, most recently from 068fe49 to 85aa7b7 Compare November 16, 2018 17:27

mlautman mentioned this pull request Nov 16, 2018

WIP: Get property type ros-visualization/rqt#160

Closed

mlautman commented Nov 19, 2018

View reviewed changes

rosidl_generator_py/test/test_interfaces.py Outdated Show resolved Hide resolved

mlautman commented Nov 19, 2018

View reviewed changes

rosidl_generator_py/test/test_interfaces.py Outdated Show resolved Hide resolved

mlautman commented Nov 19, 2018

View reviewed changes

rosidl_generator_py/test/test_interfaces.py Outdated Show resolved Hide resolved

mlautman commented Nov 19, 2018

View reviewed changes

rosidl_generator_py/test/test_interfaces.py Outdated Show resolved Hide resolved

brawner reviewed Nov 19, 2018

View reviewed changes

rosidl_generator_py/resource/_msg.py.em Outdated Show resolved Hide resolved

brawner reviewed Nov 19, 2018

View reviewed changes

mlautman force-pushed the adding-slot-types-to-python-msgs branch 2 times, most recently from e820439 to ac9c660 Compare November 19, 2018 20:29

mlautman changed the title ~~Adding a slot_type message field to python msg classes~~ Adding a get_slot_fields_and_types method to python msg classes Nov 19, 2018

Adding a get_slot_fields_and_types method to python msg classes

75d6aef

mlautman force-pushed the adding-slot-types-to-python-msgs branch from ac9c660 to 40d4bcc Compare November 19, 2018 20:48

mlautman commented Nov 19, 2018

View reviewed changes

rosidl_generator_py/resource/_msg.py.em Outdated Show resolved Hide resolved

removing noqa: W504

706a5dd

mlautman force-pushed the adding-slot-types-to-python-msgs branch from 343ce69 to 706a5dd Compare November 19, 2018 21:40

dirk-thomas merged commit fc8b868 into ros2:master Nov 19, 2018

dirk-thomas removed the in review Waiting for review (Kanban column) label Nov 19, 2018

mlautman deleted the adding-slot-types-to-python-msgs branch November 19, 2018 23:06

mlautman restored the adding-slot-types-to-python-msgs branch November 26, 2018 21:08

mlautman deleted the adding-slot-types-to-python-msgs branch November 26, 2018 21:08

mlautman restored the adding-slot-types-to-python-msgs branch November 26, 2018 21:08

mikaelarguedas mentioned this pull request Feb 17, 2019

store types as tuple and return ordered dict #30

Closed

dirk-thomas mentioned this pull request Oct 24, 2019

Add support to get message field types #16

Closed

	def get_slot_types_dict(cls):
	def get_slot_fields_and_types(cls):

Conversation

mlautman commented Nov 3, 2018

Uh oh!

mlautman commented Nov 3, 2018

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!

mlautman Nov 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

mlautman commented Nov 7, 2018

Uh oh!

Uh oh!

mlautman commented Nov 16, 2018

Uh oh!

mlautman commented Nov 16, 2018

Uh oh!

dirk-thomas commented Nov 17, 2018

Uh oh!

mlautman commented Nov 19, 2018

Uh oh!

dirk-thomas commented Nov 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlautman commented Nov 19, 2018

Uh oh!

Uh oh!

dirk-thomas commented Nov 19, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mlautman Nov 13, 2018 •

edited

Loading