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

+@[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

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

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


Suggested change
defget_slot_types_dict(cls):
defget_slot_fields_and_types(cls):

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

+
+ @@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

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 47cf04cCompare 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 9454002Compare 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

Comment threadrosidl_generator_py/resource/_msg.py.em Outdated

mlautman force-pushed the adding-slot-types-to-python-msgs branch 2 times, most recently from 068fe49 to 85aa7b7Compare 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

Comment threadrosidl_generator_py/test/test_interfaces.py Outdated

mlautman commented Nov 19, 2018
View reviewed changes

Comment threadrosidl_generator_py/test/test_interfaces.py Outdated

mlautman commented Nov 19, 2018
View reviewed changes

Comment threadrosidl_generator_py/test/test_interfaces.py Outdated

mlautman commented Nov 19, 2018
View reviewed changes

Comment threadrosidl_generator_py/test/test_interfaces.py Outdated

brawner reviewed Nov 19, 2018
View reviewed changes

Comment threadrosidl_generator_py/resource/_msg.py.em Outdated

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 ac9c660Compare 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 classesNov 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 40d4bccCompare November 19, 2018 20:48

mlautman commented Nov 19, 2018
View reviewed changes

Comment threadrosidl_generator_py/resource/_msg.py.em Outdated

removing noqa: W504
706a5dd

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

dirk-thomas merged commit fc8b868 into ros2:masterNov 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

sloretz mentioned this pull request Jul 13, 2026
fix(generator): emit .msg syntax in _fields_and_field_types for bounded types #262
Open

	defget_slot_types_dict(cls):
	defget_slot_fields_and_types(cls):

Uh oh!

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!

mlautmanNov 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

mlautmanNov 13, 2018•
edited
Loading