[rcl_lifecycle] Add warnings #830
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
fujitatomoya
approved these changes
Oct 14, 2020
|
Hi @fujitatomoya, for the next ROS2 release, we want to add several warnings to all of the maintained C and C++ packages. The thinking is that this will help maintain code quality and catch bugs (it has already caught a few in other packages). I made a proposal and worked with the team to find a set of warnings to include. I'll ask if I can make the proposal public. Below is the full set of warnings. This PR doesn't do all of them because some of the warnings require solving additional problems (so far, mostly to do with vendor packages, like googletest). |
|
sounds great 👍 thanks 😃 |
clalancette
requested changes
Oct 14, 2020
Signed-off-by: Audrow Nash <audrow.nash@gmail.com>
Signed-off-by: Audrow Nash <audrow.nash@gmail.com>
255312d to
d159f26
Compare
|
I also tested above packages to see if changing the type of
|
clalancette
approved these changes
Oct 21, 2020
skyegalaxy
pushed a commit
to irobot-ros/rcl
that referenced
this pull request
Mar 12, 2025
* basic ipc implementation from alsora/new_ipc_proposal Signed-off-by: alberto <alberto.soragna@gmail.com> better use of node_topic create subscription Signed-off-by: alberto <alberto.soragna@gmail.com> added intra process manager test Signed-off-by: alberto <alberto.soragna@gmail.com> fixed ring buffer and added test Signed-off-by: alberto <alberto.soragna@gmail.com> added intra process buffer test Signed-off-by: alberto <alberto.soragna@gmail.com> added intra process buffer test Signed-off-by: alberto <alberto.soragna@gmail.com> Signed-off-by: alberto <alberto.soragna@gmail.com> removed intra-process methods from subscription base Signed-off-by: alberto <alberto.soragna@gmail.com> using lock_guard instead of unique_lock, renamed var without camel case Signed-off-by: alberto <alberto.soragna@gmail.com> using unordered set and references in intra process manager Signed-off-by: alberto <alberto.soragna@gmail.com> subscription intra-process does not depend anymore on subscription, but has a copy of the callback Signed-off-by: alberto <alberto.soragna@gmail.com> changed buffer API to use rvo Signed-off-by: Alberto <alberto.soragna@gmail.com> avoid copying shared_ptr Signed-off-by: alberto <alberto.soragna@gmail.com> revert not needed changes to create_subscription Signed-off-by: alberto <alberto.soragna@gmail.com> updated tests according to new buffer APIs Signed-off-by: alberto <alberto.soragna@gmail.com> updated types in ring buffer implementation avoid using uint32_t Signed-off-by: alberto <alberto.soragna@gmail.com> using unique ptr for buffers in subscription_intra_process Signed-off-by: alberto <alberto.soragna@gmail.com> added missing std::move in subscription_intra_process constructor Signed-off-by: alberto <alberto.soragna@gmail.com> use consisting names for ring_buffer_implementation members Signed-off-by: alberto <alberto.soragna@gmail.com> addressing typos, one-liners and similar from ivanpauno review Signed-off-by: alberto <alberto.soragna@gmail.com> moved subscription_intra_process_base to its own files and moved non templated method from derived class Signed-off-by: alberto <alberto.soragna@gmail.com> removed forward declarations, fixed include subscription_intra_process_base Signed-off-by: alberto <alberto.soragna@gmail.com> removed member variable from do_intra_process_publish signature Signed-off-by: alberto <alberto.soragna@gmail.com> declare public before private in intra_process_manager_impl Signed-off-by: alberto <alberto.soragna@gmail.com> made matches_any_intra_process_publishers const Signed-off-by: alberto <alberto.soragna@gmail.com> using const reference in get_all_matching_publishers Signed-off-by: alberto <alberto.soragna@gmail.com> added deleter and alloc templates in intra_process_buffer Signed-off-by: alberto <alberto.soragna@gmail.com> added RCLCPP_WARN to intra_process_manager_impl Signed-off-by: alberto <alberto.soragna@gmail.com> passing context from node to subscription_intra_process Signed-off-by: alberto <alberto.soragna@gmail.com> using allocators in intra_process_manager Signed-off-by: alberto <alberto.soragna@gmail.com> use size_t instead of int in ring buffer indices Signed-off-by: alberto <alberto.soragna@gmail.com> creating buffer inside subscription_intra_process constructor Signed-off-by: alberto <alberto.soragna@gmail.com> fix lint errors Signed-off-by: alberto <alberto.soragna@gmail.com> throw error if trying to dequeue when buffer empty; remove duplicated methods in intra_process_buffer Signed-off-by: alberto <alberto.soragna@gmail.com> added todo for creating an rmw function for checking qos compatibility Signed-off-by: alberto <alberto.soragna@gmail.com> test fixes Signed-off-by: alberto <alberto.soragna@gmail.com> refactored intra_process_manager, removed ipm impl Signed-off-by: alberto <alberto.soragna@gmail.com> added mutex in intra_process_manager add_* methods Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> added allocator to intra_process_buffer Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> added invalid intra_process qos test for subscription Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> throw error if history size is 0 with keep last and ipc Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> using allocator when creating unique_ptr from shared_ptr Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> adding deleter template argument to intra_process buffer Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> fix linter Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> throw error with callbackT different from messageT Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> updated deleter template argument in subscription factory Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> Fix typo in test fixture tear down method name (ros2#787) Signed-off-by: Jacob Perron <jacob@openrobotics.org> Add free function for creating service clients (ros2#788) Equivalent to the free function for creating a service. Resolves ros2#768 Signed-off-by: Jacob Perron <jacob@openrobotics.org> Cmake infrastructure for creating components (ros2#784) *cmake macro to create components for libraries with multiple nodes Signed-off-by: Siddharth Kucheria <kucheria@usc.edu> Allow registering multiple on_parameters_set_callback (ros2#772) Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com> fix for multiple nodes not being recognized (ros2#790) Signed-off-by: Siddharth Kucheria <kucheria@usc.edu> Remove non-package from ament_target_dependencies() (ros2#793) Signed-off-by: Shane Loretz <sloretz@osrfoundation.org> fix linter issue (ros2#795) Signed-off-by: Siddharth Kucheria <kucheria@usc.edu> Make TimeSource ignore use_sim_time events coming from other nodes. (ros2#799) Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> passing deleter template parameter Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> small fixes for failing tests Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> fixed imports in test_intra_process_manager Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> using RCLCPP_SMART_PTR_ALIASES_ONLY and RCLCPP_PUBLIC macros Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> added RCLCPP_PUBLIC macros and virtual destructor to sub intra_process base Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> added unique_ptr alias to macros Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> updated test_intra_process_manager.cpp Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> remove mock msgs from rclcpp (ros2#800) Signed-off-by: Karsten Knese <karsten@openrobotics.org> Add line break after first open paren in multiline function call (ros2#785) * Add line break after first open paren in multiline function call as per developer guide: https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#open-versus-cuddled-braces see ament/ament_lint#148 Signed-off-by: Dan Rose <dan@digilabs.io> Fix dedent when first function argument starts with a brace Signed-off-by: Dan Rose <dan@digilabs.io> Line break with multiline if condition Remove line breaks where allowed. Signed-off-by: Dan Rose <dan@digilabs.io> Fixup after rebase Signed-off-by: Dan Rose <dan@digilabs.io> Fixup again after reverting indent_paren_open_brace Signed-off-by: Dan Rose <dan@digilabs.io> * Revert comment spacing change, condense some lines Signed-off-by: Dan Rose <dan@digilabs.io> Adapt to '--ros-args ... [--]'-based ROS args extraction (ros2#816) * Use --ros-args to deal with node arguments in rclcpp. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Document implicit --ros-args flag in NodeOptions::arguments(). Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Add missing size_t to int cast. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Only add implicit --ros-args flag if not present already. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Add some rclcpp::NodeOptions test coverage. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Address peer review comments. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Please cpplint and uncrustify. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> Guard against making multiple result requests for a goal handle (ros2#808) This fixes a runtime error caused by a race condition when making consecutive requests for the result. Specifically, this happens if the user provides a result callback when sending a goal and then calls async_get_result shortly after. Resolves ros2#783 Signed-off-by: Jacob Perron <jacob@openrobotics.org> Explain return value of spin_until_future_complete (ros2#792) Signed-off-by: Dan Rose <dan@digilabs.io> Allow passing logger by const ref (ros2#820) Signed-off-by: Karsten Knese <karsten@openrobotics.org> Delete unnecessary call for get_node_by_group (ros2#823) Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com> Fix get_node_interfaces functions taking a pointer (ros2#821) Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com> add callback group as member variable and constructor arg (ros2#811) Signed-off-by: bpwilcox <bpwilcox@eng.ucsd.edu> remove callback group as member variable Wrap documentation examples in code blocks (ros2#830) This makes the code examples easier to read in the generated documentation. Signed-off-by: Jacob Perron <jacob@openrobotics.org> Crash in callback group pointer vector iterator (ros2#814) Signed-off-by: Guillaume Autran <gautran@clearpath.ai> add mutex in add/remove_node and wait_for_work to protect concurrent use/change of memory_strategy_ (ros2#837) Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> Fix hang with timers in MultiThreadedExecutor (ros2#835) (ros2#836) Signed-off-by: Todd Malsbary <todd.malsbary@intel.com> Use of -r/--remap flags where appropriate. (ros2#834) Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> Force explicit --ros-args in NodeOptions::arguments(). (ros2#845) Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> Fail on invalid and unknown ROS specific arguments (ros2#842) * Fail on invalid and unknown ROS specific arguments. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Revert changes to utilities.hpp in rclcpp Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> * Fully revert change to utilities.hpp Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com> Fix typo in deprecated warning. (ros2#848) "it's" instead of its Signed-off-by: Luca Della Vedova <luca@openrobotics.org> Add throwing parameter name if parameter is not set (ros2#833) * added throwing parameter name if parameter is not set Signed-off-by: Alex <cvbn127@gmail.com> Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com> check valid timer handler 1st to reduce the time window for scan. (ros2#841) Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com> remove features and related code which were deprecated in dashing (ros2#852) Signed-off-by: William Woodall <william@osrfoundation.org> reset error message before setting a new one, embed the original one (ros2#854) Signed-off-by: Dirk Thomas <dirk-thomas@users.noreply.github.com> restored virtual destructor in publisher_base Signed-off-by: Soragna, Alberto <alberto.soragna@gmail.com> * fixup a few things after rebase Signed-off-by: William Woodall <william@osrfoundation.org> * refactor some API's and get code compiling again Signed-off-by: William Woodall <william@osrfoundation.org> * docs and style changes (whitespace) Signed-off-by: William Woodall <william@osrfoundation.org> * move new intra process internals into experimental namespace Signed-off-by: William Woodall <william@osrfoundation.org> * uncrustify Signed-off-by: William Woodall <william@osrfoundation.org> * fix issues with LoanedMessages after rebase Signed-off-by: William Woodall <william@osrfoundation.org> * more fixups Signed-off-by: William Woodall <william@osrfoundation.org> * readd logic for avoiding in compatible QoS Signed-off-by: William Woodall <william@osrfoundation.org> * avoid an error when intra process is disabled Signed-off-by: William Woodall <william@osrfoundation.org> * change intra process to preserve pointer in cyclic_pipeline Signed-off-by: William Woodall <william@osrfoundation.org> * fix issue matching topics in intra process Signed-off-by: William Woodall <william@osrfoundation.org> * fix some issues with the tests after latest behavior change Signed-off-by: William Woodall <william@osrfoundation.org> * address review feedback Signed-off-by: William Woodall <william@osrfoundation.org> * fix the initialization order Signed-off-by: William Woodall <william@osrfoundation.org> * avoid possible loss of data warning Signed-off-by: William Woodall <william@osrfoundation.org> * more fixes related to initialization Signed-off-by: William Woodall <william@osrfoundation.org> * fix use of custom allocators Signed-off-by: William Woodall <william@osrfoundation.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enables
-Wformat=2,-Wconversion,-Wshadow, and-Wsign-conversioninrcl_lifecycle. This PR relies on using gtest v1.10.0, see ament/googletest#8.