ros2 / rosidl_typesupport

Packages which provide the typesupport for ROS messages and services
Apache License 2.0
13 stars 34 forks source link

Review the contract of `get_typesupport_handle_function` #100

Open clalancette opened 3 years ago

clalancette commented 3 years ago

While working on https://github.com/ros2/rmw_cyclonedds/pull/274 and https://github.com/ros2/rmw/pull/293 , a review of the contract of get_typesupport_handle_function revealed it was less than ideal.

In particular, get_typesupport_handle_function is used to probe for the existence of typesupport libraries. If a particular implementation doesn't support that typesupport, then an rcutils error is set and nullptr is returned. If the implementation does support that typesupport, but setting it up failed, then an rcutils error is set and nullptr is returned. Finally, if the implementation does support that typesupport, and setting it up succeeded, then no rcutils error is set and a pointer is returned.

Setting an rcutils error when a typesupport doesn't exist seems to somewhat defeat the purpose of it being a "probe". It leads to code like that in https://github.com/ros2/rmw_cyclonedds/pull/274 and https://github.com/ros2/rmw_cyclonedds/pull/271, which works, but is kind of ugly.

My proposal here is that we change the contract of get_typesupport_handle_function so that if the implementation doesn't support that typesupport, then no rcutils error is set and nullptr is returned. That way, the caller can disambiguate between no support (nullptr + no rcutils error), a fatal error in setting up (nullptr + rcutils error), and success (real pointer). This change will require us to review all uses of get_typesupport_handle_function to make sure that the callers are prepared for this.