-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert all rcl_*_t types to shared pointers #1
Conversation
5eab3d7
to
247a290
Compare
Converts all rcl_*_t types in the memory allocation strategy to shared pointers to prevent crash happening when a subscriber is reset. Issue: ros2#349
247a290
to
a5c7c60
Compare
@@ -76,15 +76,15 @@ class SubscriptionBase | |||
get_topic_name() const; | |||
|
|||
RCLCPP_PUBLIC | |||
rcl_subscription_t * | |||
std::shared_ptr<rcl_subscription_t> | |||
get_subscription_handle(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These functions make up part of the public API for rclcpp - should we provide any versions that allow return type of rcl_subscription_t* to remain backwards compatible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll let OSRF decide :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor comment but non blocking for merge.
ros2#1452) * Copying files from rosbag2 The generic_* files are from rosbag2_transport typesupport_helpers incl. test is from rosbag2_cpp memory_management.hpp is from rosbag2_test_common test_pubsub.cpp was renamed from test_rosbag2_node.cpp from rosbag2_transport Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Rebrand into rclcpp_generic Add package.xml, CMakeLists.txt, Doxyfile, README.md and CHANGELOG.rst Rename namespaces Make GenericPublisher and GenericSubscription self-contained by storing shared library New create() methods that return shared pointers Add docstrings Include only what is needed Make linters & tests pass Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Review feedback * Delete CHANGELOG.rst * Enable cppcheck * Remove all references to rosbag2/ros2bag Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Move rclpp_generic into rclcpp Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Rename namespace rclcpp_generic to rclcpp::generic Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Free 'create' functions instead of static functions in class Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Remove 'generic' subdirectory and namespace hierarchy Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Order includes according to style guide Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Remove extra README.md Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Also add brief to class docs Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Make ament_index_cpp a build_depend Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Add to rclcpp.hpp Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Remove memory_management, use rclcpp::SerializedMessage in GenericPublisher::publish Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Clean up the typesupport_helpers Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Use make_shared, add UnimplementedError Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Add more comments, make member variable private, remove unnecessary include Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Apply suggestions from code review Co-authored-by: William Woodall <william+github@osrfoundation.org> Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Rename test Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Update copyright and remove ament_target_dependencies for test Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Accept PublisherOptions and SubscriptionOptions Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Remove target_include_directories Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Add explanatory comment to SubscriptionBase Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Use kSolibPrefix and kSolibExtension from rcpputils Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Fix downstream build failure by making ament_index_cpp a build_export_depend Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Use path_for_library(), fix documentation nitpicks Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Improve error handling in get_typesupport_handle Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Accept SubscriptionOptions in GenericSubscription Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Make use of PublisherOptions in GenericPublisher Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Document typesupport_helpers Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Improve documentation Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Use std::function instead of function pointer Co-authored-by: William Woodall <william+github@osrfoundation.org> Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Minimize vertical whitespace Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Add TODO for callback with message info Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Link issue in TODO Signed-off-by: Nikolai Morin <nikolai.morin@apex.ai> * Add missing include for functional Signed-off-by: nnmm <nnmmgit@gmail.com> * Fix compilation Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Fix lint Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Address review comments (#1) * fix redefinition of default template arguments Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com> * address review comments Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com> * rename test executable Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com> * add functionality to lifecycle nodes Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com> * Refactor typesupport helpers * Make extract_type_identifier function private * Remove unused extract_type_and_package function * Update unit tests Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Remove note about ament from classes This comment only applies to the free functions. Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Fix formatting Co-authored-by: Karsten Knese <Karsten1987@users.noreply.github.com> * Fix warning Possible loss of data from double to rcutils_duration_value_t Signed-off-by: Jacob Perron <jacob@openrobotics.org> * Add missing visibility macros Signed-off-by: Jacob Perron <jacob@openrobotics.org> Co-authored-by: William Woodall <william+github@osrfoundation.org> Co-authored-by: Jacob Perron <jacob@openrobotics.org> Co-authored-by: Karsten Knese <Karsten1987@users.noreply.github.com>
Converts all rcl_*_t types in the memory allocation strategy to shared pointers to prevent crash happening when a subscriber is reset.
Issue: ros2#349