ros2 / rmw_fastrtps

Implementation of the ROS Middleware (rmw) Interface using eProsima's Fast RTPS.
Apache License 2.0
157 stars 117 forks source link

Improve the implementation of incompatible type #675

Open clalancette opened 1 year ago

clalancette commented 1 year ago

As part of the review for #654, there was a suggestion on how to further improve the code. Given how long that PR had been under review and how close we were to the deadline, we decided to punt that improvement until a later iteration. Here's the suggested improvement from @iuhilnehc-ynos :

Sorry for the late review and it's not a blocking comment/question as well.

Can we add a new struct derived from eprosima::fastdds::dds::Topic to keep the EventListenerInterface as a member inside,
and then use two dynamic_cast operations for checking the new struct and Topic by order in remove_topic_and_type?

something might be done as below,

add overloaded functions for CustomParticipantInfo::find_or_create_topic and CustomParticipantInfo::delete_topic for publisher and subscription

so we can get the following benefit

  1.  the client and service can take the original source code if they don't care about the event_listener.
  2. there is no `rmw_fastrtps_shared_cpp::remove_topic_and_type( 'info->publisher_event_ / nullptr' ) with an extra parameter unrelated to the method name called.