ros2 / rosidl_python

rosidl support for Python
Apache License 2.0
20 stars 45 forks source link

Derive generated message classes from a base class #215

Open EricGallimore opened 1 month ago

EricGallimore commented 1 month ago

In ROS1, all generated message classes derived from a common class: genpy.message.Message.

This allows for a bunch of handy behavior, including the ability to use isinstance(..., genpy.message.Message) to check if an object is a ROS message, and it allows for meaningful type hints (rather than using typing.Any, as is usually done for ROS2 messages at the moment).

I propose having all generated ROS2 message classes derive from a new RosMessage class to restore this functionality. (Services and actions should probably also get similar treatment at some point).

Making the generated classes inherit from the RosMessage class is trivial; it's just a one-line change to the generator template (_msg.py.em).

Additionally, the definition of the base class is simple. It would be useful even if it were an empty class, but it should probably include a definition for the get_fields_and_field_types classmethod.

What I'm stuck on right now is where the base class should go, since it needs to be available at runtime. This seems to get messy: the rosidl_python package is only used for building messages, so presumably it shouldn't go in here. I'm leaning toward rosidl/rosidl_pycommon, but would appreciate guidance from someone who better understands the design philosophy here.

If someone can point me in the right direction, I'll make some PR(s).

MichaelOrlov commented 1 month ago

@wjwwood Friendly ping to comment on this issue.

InvincibleRMC commented 1 week ago

This might not be the solution you want but, I came across this problem and create a solution which worked for me. I created a Python Protocol named Msg to use as a type hints for message objects. It can be found here. Currently it is only has the type_support attributes since that was all I needed at the time but could be updated to have all of the default functions if required. One thing to note, is this currently only exists in rolling and likely wont be back-ported.