Discussion about how to add a new PRIMITIVE_TYPES to rosidl(msg)

Next Generation ROS
1

Hi , I’m working on adding a new type (e.g. new_uint8) to PRIMITIVE_TYPES in rosidl , cause i want to do some extra operations on the uint8[] data but i don’t want to effect the original uint8 type

For example , in rosidl/rosidl_generator_py/resource/_msg_support.c.em , i can change the new_uint8 type to bytes instead of list when i received the msgs

I have looked some relative codes , and i find this in rosidl/rosidl_parser/rosidl_parser/__init__.py

PRIMITIVE_TYPES = [
‘bool’,
‘byte’,
‘char’,
# TODO reconsider wchar
‘float32’,
‘float64’,
‘int8’,
‘uint8’,
‘int16’,
‘uint16’,
‘int32’,
‘uint32’,
‘int64’,
‘uint64’,
‘string’,
# TODO reconsider wstring / u16string / u32string
# TODO duration and time
‘duration’, # for compatibility only
‘time’, # for compatibility only
]

So i guess it’s possible to add a new type

But i don’t know whether there are other codes i need to consider , besides the rosidl_parser and /rosidl_generator_py/resource/_msg_support.c.em

Looking forward to some suggestions !
Thanks !

2

Maybe you can elaborate what exactly you are trying to achieve. The current format supports basically all primitive types. Your use case seem to be already addressed by defining a field with the type byte[]. Maybe for that type it only needs to be reconsidered how it is mapped into specific languages like Python. I agree with you that bytes sounds like the more “natural” type in Python.

3

Maybe you can elaborate what exactly you are trying to achieve

when I use cv_bridge in ros2 , the img_msg is turned into list in python , but the cv_bridge needs a byte type when transform the img_msg.data into cv2 img type .
(the img_msg.data is uint8 [] and is turned into list after transform)

So we rewrite the genorator_py in rosidl to make the uint8 turn into byte type , and it works well

but to avoid effecting other algorithm which using uint8 as well in pyhton , we want to create a new_uint8[] for a new msg type like new_imagemsg so that we don’t need to transform all uint8[] to bytes , but only in some special situations (like using cv_bridge) , we can use the new message type and get the byte type instead of list . and other algorithm can still receive the image_msg.data as list type

So i just want to know whether this idea can work , and how to make it .

I’m trying to rewrite the python-end in rosidl and build a new primitive type （maybe use dtype() ?） and a new msg_type and try to analyze the type

4

I don’t think “just adding a new type” is the right way here. A patch adding new_uint8 would also not be accepted in a PR.

Instead the existing mapping (see http://design.ros2.org/articles/generated_interfaces_python.html) should be re-evaluated. In that process all other language mappings need to be considered too.

As mentioned above I am in favor of mapping byte[] In Python to bytes instead of a list of integers.

5

Currently, in Python, byte[] maps to a list of builtin.bytes (each list element being of length 1). And I agreed with you it could be just builtin.bytes and not a list.

My understanding is that the proposal from @Zongpei_Jiang is to map uint8[] to builtin.bytes as several ROS 1 messages use uint8 to send binary data around. So this is a different proposal I think, and I’m not sure that this should map to builtin.bytes in that case.

:+1: that’s been on our list for a long time but we didn’t get around doing it yet.

6

My understanding is that the proposal from @Zongpei_Jiang is to map uint8 to builtin.bytes as several ROS 1 messages use uint8 to send binary data around

I agree with that . Just map all the uint8[] to bytes[] instead of list[] may cause other problems since some algorithms are designed using the msg.data as list in python .

So that’s why i want to create a type like imguint8 that i can also create a BytesImage.msg which use the type to deliver image messages like what ‘Image.msg’ does . And i have changed some files in rosidl package and sensor_msg package and succeeded in ament building .

I have changed following files to add the imguint8:

rosidl/rosidl_generator_py/resource/_msg_support.c.em
rosidl/rosidl_generator_py/resource/_msg.py.em
rosidl/rosidl_generator_py/msg/Primitives.msg
rosidl/rosidl_generator_py/rosidl_generator_py/generate_py_impl.py
rosidl/rosidl_parser/rosidl_parser/__init__.py
common_interfaces/sensor_msgs/msg/BytesImage.msg
common_interfaces/sensor_msgs/CMakeLists.txt
rosidl/rosidl_generator_c/include/rosidl_generator_c/primitives_array.h
rosidl/rosidl_generator_c/include/rosidl_generator_c/primitives_array_functions.h
rosidl/rosidl_generator_c/msg/ImgUint8.msg

rosidl/rosidl_generator_c/msg/Primitive*.msg

rosidl/rosidl_generator_c/resource/msg__struct.h.em

rosidl/rosidl_generator_c/rosidl_generator_c/__init__.py
rosidl/rosidl_generator_c/src/primitives_array_functions.c


rosidl/rosidl_typesupport_introspection_c/include/rosidl_typesupport_introspection_c/field_types.h

rosidl/rosidl_generator_cpp/msg/Primitive*.msg

rosidl/rosidl_generator_cpp/resource/msg__struct.hpp.em
rosidl/rosidl_generator_cpp/rosidl_generator_cpp/__init__.py

rosidl/rosidl_typesupport_introspection_cpp/include/rosidl_typesupport_introspection_cpp/field_types.hpp

But i failed using the new type to publish the BytesImage.msg type with reporting :
Assertion convert_from_py != NULL && “unable to retrieve convert_from_py function, type_support mustn’t have been imported”’ failed.`

So i want to know whether i can add the imguint8 type and if i can , what i have missed to make the BytesImage.msg can work .

Thanks

7

I find that maybe I should try to add a Builtin_type to builtin_interface instead of adding a primitive type directly .(like Duration and time)

8

The primitive type should not by specific to what “semantic” data you put in. Therefore img is in my opinion a no-go. The same type could also store other binary data.

That is why I recommended using byte[].

9

Blockquote[quote=“Zongpei_Jiang, post:6, topic:4607”]
I agree with that . Just map all the uint8[] to bytes[] instead of list[] may cause other problems since some algorithms are designed using the msg.data as list in python .
[/quote]

I still think that using bytes rather than list[byte] would be a good move (at least from IDL byte[] types, if not for uint8[] types). Yes, it would be a breaking change, but it would be a good breaking change and most dependent code would be very easy to update (and likely get a nice speed boost).

10

Agreed :+1:
We are planning on revisiting the mapping of all primitive types as we migrate to IDL 4.2 later this release cycle, will make sure to take all the feedback from this thread into account when doing so.

11

Just mention the current state: you can already use byte[] in a .msg file and the resulting field in Python will be of type bytes (see rosidl_python/rosidl_generator_py/rosidl_generator_py/generate_py_impl.py at 3b97dc828ad14e3360ac52a1ef90ff335ce65715 · ros2/rosidl_python · GitHub).