REP-2004: Package Quality Categories #218

wjwwood · 2019-12-17T23:29:38Z

Abstract

This REP describes a set of categories which are meant to convey the quality, or at least the maturity, of packages in the ROS ecosystem.
Inclusion in one category or another is based on the policies to which the packages adhere.
The categories are meant to give some expectation as to the quality of a package and allows the maintainers to be more strict with some packages and less so with others.

The purpose of these categories is not to enforce quality, but to set expectations for consumers of the packages and to encourage maintainers of the packages to document how their package's policies achieve that quality level.
The documented policies allow consumers of the packages to consider any caveats for the package or its dependencies when deciding whether or not the package meets the standards for their project.

See this pull request for previous discussions on this topic, as this REP evolved from the ROS 2 "developer guidelines": ros2/ros2_documentation#460

Signed-off-by: William Woodall <william@osrfoundation.org>

…lows one per person Signed-off-by: William Woodall <william@osrfoundation.org>

gavanderhoorn · 2019-12-18T08:43:39Z

rep-2004.rst

+Backwards Compatibility
+=======================
+
+Since there are no preceeding standards in this space, at least in the ROS community, of which we are aware, there are no backwards compatibility concerns.


It may not have been as rigorously standardised in a REP, but at least in ROS-Industrial we've had the Software Status indicators for quite a while now.

These are not a bw-compatibility concern, but I believe they do qualify as existing work/prior art.

Ah thanks, I looked briefly for this (I remembered something from ROS-I) but couldn't find it (I think I was looking for the 'quality' keyword). My mistake. I'll include it here, but as you say I don't think they're incompatible or affect backwards compatibility. I'll also read up on this wiki page and see if anything should be incorporated into this REP from it.

vmayoral · 2019-12-18T09:28:00Z

rep-2004.rst

+  * Linters and Static Analysis
+
+    * Must have a code style and enforce it.
+    * Must use static analysis tools where applicable.


How about considering the static analysis results as part of the quality qualification? (If it's somewhere here I missed it). E.g., cppcheck and other static analysis report on the severity of the bugs. Would it make sense to consider bugs above certain severity as a requirement for qualifying for this level? If so, what're the requirements for belonging to other levels, e.g. Quality Level 2? I'm happy to make a proposal myself if this is of interest but I'd like to check before puting time on it.

This would make sense to me as well. It's going to be difficult to figure out a level of sufficient compliance, but if we're running the tools (as this quality level requires), then it would be good to have some idea of what a minimally acceptable result of running the tools should be.

My thought was you have to pass all the checks... But I guess level 2 or 3 could be that you're running them, but ignoring some failures. This really depends on what the tools are measuring and whether or not there are false positives. Unlike coverage or performance (where the things being measured are relatively universal across languages and tools), static analysis and linting are much more varied, so I think it will be hard to put a quantitative requirement here, other than 100% compliance or no compliance requirement.

Based on the (I believe positive) reactions, I've invested a bit of additional time on this and put together some more specific suggestions. Proposals are underlined. Find below a few additional comments and ideas that @wjwwood and others in position to integrate may want to consider for this "Package Quality Categories":

use of cppcheck for quality qualification:

cppcheck output can be used to determine bugs that may affect the overall quality of the software. See below an example of how cppcheck is used over the rclcpp ROS 2 package:

cppcheck output of rclcpp ROS 2 package

cppcheck --force --enable=all . --output-file=cppcheck_output Checking rclcpp/src/rclcpp/any_executable.cpp ... 1/118 files checked 0% done Checking rclcpp/src/rclcpp/callback_group.cpp ... 2/118 files checked 1% done Checking rclcpp/src/rclcpp/client.cpp ... 3/118 files checked 2% done Checking rclcpp/src/rclcpp/clock.cpp ... 4/118 files checked 3% done Checking rclcpp/src/rclcpp/context.cpp ... 5/118 files checked 4% done Checking rclcpp/src/rclcpp/contexts/default_context.cpp ... 6/118 files checked 5% done Checking rclcpp/src/rclcpp/detail/rmw_implementation_specific_payload.cpp ... 7/118 files checked 5% done Checking rclcpp/src/rclcpp/detail/rmw_implementation_specific_publisher_payload.cpp ... 8/118 files checked 6% done Checking rclcpp/src/rclcpp/detail/rmw_implementation_specific_subscription_payload.cpp ... 9/118 files checked 7% done Checking rclcpp/src/rclcpp/duration.cpp ... 10/118 files checked 8% done Checking rclcpp/src/rclcpp/event.cpp ... 11/118 files checked 9% done Checking rclcpp/src/rclcpp/exceptions.cpp ... 12/118 files checked 10% done Checking rclcpp/src/rclcpp/executor.cpp ... 13/118 files checked 11% done Checking rclcpp/src/rclcpp/executors.cpp ... 14/118 files checked 11% done Checking rclcpp/src/rclcpp/executors/multi_threaded_executor.cpp ... 15/118 files checked 12% done Checking rclcpp/src/rclcpp/executors/single_threaded_executor.cpp ... 16/118 files checked 13% done Checking rclcpp/src/rclcpp/expand_topic_or_service_name.cpp ... 17/118 files checked 14% done Checking rclcpp/src/rclcpp/graph_listener.cpp ... 18/118 files checked 15% done Checking rclcpp/src/rclcpp/init_options.cpp ... 19/118 files checked 16% done Checking rclcpp/src/rclcpp/intra_process_manager.cpp ... 20/118 files checked 16% done Checking rclcpp/src/rclcpp/logger.cpp ... Checking rclcpp/src/rclcpp/logger.cpp: RCLCPP_LOGGING_ENABLED... 21/118 files checked 17% done Checking rclcpp/src/rclcpp/memory_strategies.cpp ... 22/118 files checked 18% done Checking rclcpp/src/rclcpp/memory_strategy.cpp ... 23/118 files checked 19% done Checking rclcpp/src/rclcpp/node.cpp ... 24/118 files checked 20% done Checking rclcpp/src/rclcpp/node_interfaces/node_base.cpp ... 25/118 files checked 21% done Checking rclcpp/src/rclcpp/node_interfaces/node_clock.cpp ... 26/118 files checked 22% done Checking rclcpp/src/rclcpp/node_interfaces/node_graph.cpp ... 27/118 files checked 22% done Checking rclcpp/src/rclcpp/node_interfaces/node_logging.cpp ... 28/118 files checked 23% done Checking rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp ... 29/118 files checked 24% done Checking rclcpp/src/rclcpp/node_interfaces/node_services.cpp ... 30/118 files checked 25% done Checking rclcpp/src/rclcpp/node_interfaces/node_time_source.cpp ... 31/118 files checked 26% done Checking rclcpp/src/rclcpp/node_interfaces/node_timers.cpp ... 32/118 files checked 27% done Checking rclcpp/src/rclcpp/node_interfaces/node_topics.cpp ... 33/118 files checked 27% done Checking rclcpp/src/rclcpp/node_interfaces/node_waitables.cpp ... 34/118 files checked 28% done Checking rclcpp/src/rclcpp/node_options.cpp ... Checking rclcpp/src/rclcpp/node_options.cpp: _WIN32... 35/118 files checked 29% done Checking rclcpp/src/rclcpp/parameter.cpp ... 36/118 files checked 30% done Checking rclcpp/src/rclcpp/parameter_client.cpp ... 37/118 files checked 31% done Checking rclcpp/src/rclcpp/parameter_events_filter.cpp ... 38/118 files checked 32% done Checking rclcpp/src/rclcpp/parameter_map.cpp ... 39/118 files checked 33% done Checking rclcpp/src/rclcpp/parameter_service.cpp ... 40/118 files checked 33% done Checking rclcpp/src/rclcpp/parameter_value.cpp ... 41/118 files checked 34% done Checking rclcpp/src/rclcpp/publisher_base.cpp ... 42/118 files checked 35% done Checking rclcpp/src/rclcpp/qos.cpp ... 43/118 files checked 36% done Checking rclcpp/src/rclcpp/qos_event.cpp ... 44/118 files checked 37% done Checking rclcpp/src/rclcpp/service.cpp ... 45/118 files checked 38% done Checking rclcpp/src/rclcpp/signal_handler.cpp ... Checking rclcpp/src/rclcpp/signal_handler.cpp: ANDROID;_GNU_SOURCE... Checking rclcpp/src/rclcpp/signal_handler.cpp: RCLCPP_HAS_SIGACTION... Checking rclcpp/src/rclcpp/signal_handler.cpp: _WIN32... Checking rclcpp/src/rclcpp/signal_handler.cpp: __APPLE__... 46/118 files checked 38% done Checking rclcpp/src/rclcpp/subscription_base.cpp ... 47/118 files checked 39% done Checking rclcpp/src/rclcpp/subscription_intra_process_base.cpp ... 48/118 files checked 40% done Checking rclcpp/src/rclcpp/time.cpp ... 49/118 files checked 41% done Checking rclcpp/src/rclcpp/time_source.cpp ... 50/118 files checked 42% done Checking rclcpp/src/rclcpp/timer.cpp ... 51/118 files checked 43% done Checking rclcpp/src/rclcpp/type_support.cpp ... 52/118 files checked 44% done Checking rclcpp/src/rclcpp/utilities.cpp ... Checking rclcpp/src/rclcpp/utilities.cpp: _WIN32... Checking rclcpp/src/rclcpp/utilities.cpp: __APPLE__... 53/118 files checked 44% done Checking rclcpp/src/rclcpp/waitable.cpp ... 54/118 files checked 45% done Checking rclcpp/test/executors/test_multi_threaded_executor.cpp ... Checking rclcpp/test/executors/test_multi_threaded_executor.cpp: __linux__... 55/118 files checked 46% done Checking rclcpp/test/node_interfaces/test_does_not_compile/get_node_topics_interface_const_ptr_rclcpp_node.cpp ... 56/118 files checked 47% done Checking rclcpp/test/node_interfaces/test_does_not_compile/get_node_topics_interface_const_ptr_wrapped_node.cpp ... 57/118 files checked 48% done Checking rclcpp/test/node_interfaces/test_does_not_compile/get_node_topics_interface_const_ref_rclcpp_node.cpp ... 58/118 files checked 49% done Checking rclcpp/test/node_interfaces/test_does_not_compile/get_node_topics_interface_const_ref_wrapped_node.cpp ... 59/118 files checked 50% done Checking rclcpp/test/node_interfaces/test_get_node_interfaces.cpp ... 60/118 files checked 50% done Checking rclcpp/test/test_client.cpp ... 61/118 files checked 51% done Checking rclcpp/test/test_create_timer.cpp ... 62/118 files checked 52% done Checking rclcpp/test/test_duration.cpp ... 63/118 files checked 53% done Checking rclcpp/test/test_executor.cpp ... 64/118 files checked 54% done Checking rclcpp/test/test_expand_topic_or_service_name.cpp ... 65/118 files checked 55% done Checking rclcpp/test/test_externally_defined_services.cpp ... 66/118 files checked 55% done Checking rclcpp/test/test_find_weak_nodes.cpp ... 67/118 files checked 56% done Checking rclcpp/test/test_function_traits.cpp ... 68/118 files checked 57% done Checking rclcpp/test/test_init.cpp ... 69/118 files checked 58% done Checking rclcpp/test/test_intra_process_buffer.cpp ... 70/118 files checked 59% done Checking rclcpp/test/test_intra_process_manager.cpp ... 71/118 files checked 60% done Checking rclcpp/test/test_loaned_message.cpp ... 72/118 files checked 61% done Checking rclcpp/test/test_logger.cpp ... 73/118 files checked 61% done Checking rclcpp/test/test_logging.cpp ... 74/118 files checked 62% done Checking rclcpp/test/test_node.cpp ... 75/118 files checked 63% done Checking rclcpp/test/test_node_global_args.cpp ... 76/118 files checked 64% done Checking rclcpp/test/test_node_options.cpp ... 77/118 files checked 65% done Checking rclcpp/test/test_parameter.cpp ... 78/118 files checked 66% done Checking rclcpp/test/test_parameter_client.cpp ... 79/118 files checked 66% done Checking rclcpp/test/test_parameter_events_filter.cpp ... 80/118 files checked 67% done Checking rclcpp/test/test_parameter_map.cpp ... 81/118 files checked 68% done Checking rclcpp/test/test_publisher.cpp ... 82/118 files checked 69% done Checking rclcpp/test/test_publisher_subscription_count_api.cpp ... 83/118 files checked 70% done Checking rclcpp/test/test_rate.cpp ... 84/118 files checked 71% done Checking rclcpp/test/test_ring_buffer_implementation.cpp ... 85/118 files checked 72% done Checking rclcpp/test/test_serialized_message_allocator.cpp ... 86/118 files checked 72% done Checking rclcpp/test/test_service.cpp ... 87/118 files checked 73% done Checking rclcpp/test/test_subscription.cpp ... 88/118 files checked 74% done Checking rclcpp/test/test_subscription_publisher_count_api.cpp ... 89/118 files checked 75% done Checking rclcpp/test/test_subscription_traits.cpp ... 90/118 files checked 76% done Checking rclcpp/test/test_time.cpp ... 91/118 files checked 77% done Checking rclcpp/test/test_time_source.cpp ... 92/118 files checked 77% done Checking rclcpp/test/test_timer.cpp ... 93/118 files checked 78% done Checking rclcpp/test/test_utilities.cpp ... 94/118 files checked 79% done Checking rclcpp_action/src/client.cpp ... 95/118 files checked 80% done Checking rclcpp_action/src/qos.cpp ... 96/118 files checked 81% done Checking rclcpp_action/src/server.cpp ... 97/118 files checked 82% done Checking rclcpp_action/src/server_goal_handle.cpp ... 98/118 files checked 83% done Checking rclcpp_action/src/types.cpp ... 99/118 files checked 83% done Checking rclcpp_action/test/test_client.cpp ... 100/118 files checked 84% done Checking rclcpp_action/test/test_server.cpp ... 101/118 files checked 85% done Checking rclcpp_action/test/test_traits.cpp ... 102/118 files checked 86% done Checking rclcpp_components/src/component_container.cpp ... 103/118 files checked 87% done Checking rclcpp_components/src/component_container_mt.cpp ... 104/118 files checked 88% done Checking rclcpp_components/src/component_manager.cpp ... 105/118 files checked 88% done Checking rclcpp_components/test/components/test_component.cpp ... 106/118 files checked 89% done Checking rclcpp_components/test/test_component_manager.cpp ... 107/118 files checked 90% done Checking rclcpp_components/test/test_component_manager_api.cpp ... 108/118 files checked 91% done Checking rclcpp_lifecycle/src/lifecycle_node.cpp ... 109/118 files checked 92% done Checking rclcpp_lifecycle/src/node_interfaces/lifecycle_node_interface.cpp ... 110/118 files checked 93% done Checking rclcpp_lifecycle/src/state.cpp ... 111/118 files checked 94% done Checking rclcpp_lifecycle/src/transition.cpp ... 112/118 files checked 94% done Checking rclcpp_lifecycle/test/test_callback_exceptions.cpp ... 113/118 files checked 95% done Checking rclcpp_lifecycle/test/test_lifecycle_node.cpp ... 114/118 files checked 96% done Checking rclcpp_lifecycle/test/test_register_custom_callbacks.cpp ... 115/118 files checked 97% done Checking rclcpp_lifecycle/test/test_state_machine_info.cpp ... 116/118 files checked 98% done Checking rclcpp_lifecycle/test/test_state_wrapper.cpp ... 117/118 files checked 99% done Checking rclcpp_lifecycle/test/test_transition_wrapper.cpp ... 118/118 files checked 100% done root@robot:/opt/eloquent_ws/src/ros2/rclcpp# cat cppcheck_output [rclcpp/src/rclcpp/executor.cpp:566] -> [rclcpp/src/rclcpp/executor.cpp:569]: (style) Variable 'success' is reassigned a value before the old one has been used. [rclcpp/src/rclcpp/node_options.cpp:50]: (warning) Assignment of function parameter has no effect outside the function. Did you forget dereferencing it? [rclcpp/src/rclcpp/node_options.cpp:316] -> [rclcpp/src/rclcpp/node_options.cpp:319]: (style) Variable 'ros_domain_id' is reassigned a value before the old one has been used. [rclcpp/test/test_intra_process_manager.cpp:148]: (warning) Member variable 'IntraProcessBuffer::message_ptr' is not initialized in the constructor. [rclcpp/test/test_subscription.cpp:55]: (performance) Function parameter 'description' should be passed by reference. [rclcpp/test/test_subscription_traits.cpp:33]: (performance) Function parameter 'unused' should be passed by reference. [rclcpp/test/test_subscription_traits.cpp:43]: (performance) Function parameter 'unused' should be passed by reference. [rclcpp/test/test_time_source.cpp:129]: (performance) Function parameter 'clock' should be passed by reference. [rclcpp_action/test/test_client.cpp:333]: (style) Variable 'goal_response_received' is assigned a value that is never used. [rclcpp_action/test/test_client.cpp:368]: (style) Variable 'feedback_count' is modified but its new value is never used. [rclcpp_action/test/test_client.cpp:403]: (style) Variable 'result_callback_received' is assigned a value that is never used. [rclcpp_action/test/test_client.cpp:445]: (style) Variable 'result_callback_received' is assigned a value that is never used. [rclcpp_action/test/test_client.cpp:497]: (style) Variable 'cancel_response_received' is assigned a value that is never used. [rclcpp_components/src/component_manager.hpp:59]: (style) Class 'ComponentManager' has a constructor with 1 argument that is not explicit. [rclcpp_components/src/component_manager.hpp:74]: (performance) Function parameter 'request_header' should be passed by reference. [rclcpp_components/src/component_manager.hpp:75]: (performance) Function parameter 'request' should be passed by reference. [rclcpp_components/src/component_manager.hpp:80]: (performance) Function parameter 'request_header' should be passed by reference. [rclcpp_components/src/component_manager.hpp:81]: (performance) Function parameter 'request' should be passed by reference. [rclcpp_components/src/component_manager.hpp:86]: (performance) Function parameter 'request_header' should be passed by reference. [rclcpp_components/src/component_manager.hpp:87]: (performance) Function parameter 'request' should be passed by reference. [rclcpp_components/src/component_manager.cpp:121]: (performance) Function parameter 'request_header' should be passed by reference. [rclcpp_components/src/component_manager.cpp:122]: (performance) Function parameter 'request' should be passed by reference. [rclcpp_components/src/component_manager.cpp:224]: (performance) Function parameter 'request_header' should be passed by reference. [rclcpp_components/src/component_manager.cpp:225]: (performance) Function parameter 'request' should be passed by reference. [rclcpp_components/src/component_manager.cpp:249]: (performance) Function parameter 'request_header' should be passed by reference. [rclcpp_components/src/component_manager.cpp:250]: (performance) Function parameter 'request' should be passed by reference. [rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp:58]: (performance) Function parameter 'node_base_interface' should be passed by reference. [rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp:59]: (performance) Function parameter 'node_services_interface' should be passed by reference. [rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp:201]: (performance) Function parameter 'header' should be passed by reference. [rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp:202]: (performance) Function parameter 'req' should be passed by reference. [rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp:242]: (performance) Function parameter 'header' should be passed by reference. [rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp:243]: (performance) Function parameter 'req' should be passed by reference. [rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp:258]: (performance) Function parameter 'header' should be passed by reference. [rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp:259]: (performance) Function parameter 'req' should be passed by reference. [rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp:278]: (performance) Function parameter 'header' should be passed by reference. [rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp:279]: (performance) Function parameter 'req' should be passed by reference. [rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp:304]: (performance) Function parameter 'header' should be passed by reference. [rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp:305]: (performance) Function parameter 'req' should be passed by reference. [rclcpp_lifecycle/test/test_callback_exceptions.cpp:42]: (performance) Function parameter 'node_name' should be passed by reference. [rclcpp_lifecycle/test/test_callback_exceptions.cpp:88]: (performance) Function parameter 'node_name' should be passed by reference. [rclcpp_lifecycle/test/test_lifecycle_node.cpp:42]: (performance) Function parameter 'node_name' should be passed by reference. [rclcpp_lifecycle/test/test_lifecycle_node.cpp:66]: (performance) Function parameter 'node_name' should be passed by reference. [rclcpp_lifecycle/test/test_register_custom_callbacks.cpp:42]: (performance) Function parameter 'node_name' should be passed by reference. [rclcpp_lifecycle/test/test_state_wrapper.cpp:42]: (style) Variable 'state_name' is assigned a value that is never used. [rclcpp_lifecycle/test/test_transition_wrapper.cpp:46]: (style) Variable 'transition_name' is assigned a value that is never used. [rclcpp/test/test_client.cpp:33]: (style) The function 'SetUp' is never used. [rclcpp/test/executors/test_multi_threaded_executor.cpp:35]: (style) The function 'SetUpTestCase' is never used. [rclcpp/test/test_client.cpp:38]: (style) The function 'TearDown' is never used. [rclcpp/test/node_interfaces/test_get_node_interfaces.cpp:37]: (style) The function 'TearDownTestCase' is never used. [rclcpp/src/rclcpp/utilities.cpp:177]: (style) The function 'get_c_string' is never used. [rclcpp/src/rclcpp/time.cpp:247]: (style) The function 'operator+' is never used. [rclcpp/test/test_publisher.cpp:58]: (style) The function 'operator<<' is never used. [rclcpp/test/test_intra_process_manager.cpp:232]: (style) The function 'provide_intra_process_message' is never used. [rclcpp/src/rclcpp/utilities.cpp:46]: (style) The function 'signal_handlers_installed' is never used. (information) Cppcheck cannot find all the include files (use --check-config for details)

The output of cppcheck provides information about the (severity) of each reported (potential) flaw. While style ones might be disregarded, one could argue that (e.g.) a Level 1 ROS 2 quality package needs to have no (unless accepted as such) warning or error coming out of cppcheck. Another alternative is to split enforcement of the cppcheck output across levels and assign (warnings) acceptable within Level 2, but not within Level 1.

use of flawfinder for quality qualification:

With a similar rationale, flawfinder can help identify flaws. See below for the output of flawfinder on the same ROS 2 package:

flawfinder output of rclcpp ROS 2 package

root@robot:/opt/eloquent_ws/src/ros2/rclcpp# flawfinder . Flawfinder version 2.0.10, (C) 2001-2019 David A. Wheeler. Number of rules (primarily dangerous function names) in C/C++ ruleset: 223 Warning: Skipping directory with initial dot ./.git Examining ./rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp Examining ./rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_publisher.hpp Examining ./rclcpp_lifecycle/include/rclcpp_lifecycle/node_interfaces/lifecycle_node_interface.hpp Examining ./rclcpp_lifecycle/include/rclcpp_lifecycle/transition.hpp Examining ./rclcpp_lifecycle/include/rclcpp_lifecycle/state.hpp Examining ./rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node_impl.hpp Examining ./rclcpp_lifecycle/include/rclcpp_lifecycle/visibility_control.h Examining ./rclcpp_lifecycle/include/rclcpp_lifecycle/type_traits/is_manageable_node.hpp Examining ./rclcpp_lifecycle/src/state.cpp Examining ./rclcpp_lifecycle/src/lifecycle_node_interface_impl.hpp Examining ./rclcpp_lifecycle/src/node_interfaces/lifecycle_node_interface.cpp Examining ./rclcpp_lifecycle/src/lifecycle_node.cpp Examining ./rclcpp_lifecycle/src/transition.cpp Examining ./rclcpp_lifecycle/test/test_state_wrapper.cpp Examining ./rclcpp_lifecycle/test/test_callback_exceptions.cpp Examining ./rclcpp_lifecycle/test/test_state_machine_info.cpp Examining ./rclcpp_lifecycle/test/test_lifecycle_node.cpp Examining ./rclcpp_lifecycle/test/test_register_custom_callbacks.cpp Examining ./rclcpp_lifecycle/test/test_transition_wrapper.cpp Examining ./rclcpp_action/include/rclcpp_action/client_goal_handle.hpp Examining ./rclcpp_action/include/rclcpp_action/create_server.hpp Examining ./rclcpp_action/include/rclcpp_action/exceptions.hpp Examining ./rclcpp_action/include/rclcpp_action/types.hpp Examining ./rclcpp_action/include/rclcpp_action/server_goal_handle.hpp Examining ./rclcpp_action/include/rclcpp_action/visibility_control.hpp Examining ./rclcpp_action/include/rclcpp_action/qos.hpp Examining ./rclcpp_action/include/rclcpp_action/rclcpp_action.hpp Examining ./rclcpp_action/include/rclcpp_action/server.hpp Examining ./rclcpp_action/include/rclcpp_action/client_goal_handle_impl.hpp Examining ./rclcpp_action/include/rclcpp_action/create_client.hpp Examining ./rclcpp_action/include/rclcpp_action/client.hpp Examining ./rclcpp_action/src/qos.cpp Examining ./rclcpp_action/src/server.cpp Examining ./rclcpp_action/src/server_goal_handle.cpp Examining ./rclcpp_action/src/client.cpp Examining ./rclcpp_action/src/types.cpp Examining ./rclcpp_action/test/test_traits.cpp Examining ./rclcpp_action/test/test_server.cpp Examining ./rclcpp_action/test/test_client.cpp Examining ./rclcpp_components/include/rclcpp_components/node_factory_template.hpp Examining ./rclcpp_components/include/rclcpp_components/register_node_macro.hpp Examining ./rclcpp_components/include/rclcpp_components/node_instance_wrapper.hpp Examining ./rclcpp_components/include/rclcpp_components/node_factory.hpp Examining ./rclcpp_components/src/component_manager.hpp Examining ./rclcpp_components/src/component_container.cpp Examining ./rclcpp_components/src/component_container_mt.cpp Examining ./rclcpp_components/src/component_manager.cpp Examining ./rclcpp_components/test/test_component_manager.cpp Examining ./rclcpp_components/test/test_component_manager_api.cpp Examining ./rclcpp_components/test/components/test_component.cpp Warning: Skipping directory with initial dot ./.github Examining ./rclcpp/include/rclcpp/any_executable.hpp Examining ./rclcpp/include/rclcpp/parameter_client.hpp Examining ./rclcpp/include/rclcpp/time.hpp Examining ./rclcpp/include/rclcpp/macros.hpp Examining ./rclcpp/include/rclcpp/expand_topic_or_service_name.hpp Examining ./rclcpp/include/rclcpp/node_impl.hpp Examining ./rclcpp/include/rclcpp/create_publisher.hpp Examining ./rclcpp/include/rclcpp/parameter_service.hpp Examining ./rclcpp/include/rclcpp/function_traits.hpp Examining ./rclcpp/include/rclcpp/executor.hpp Examining ./rclcpp/include/rclcpp/contexts/default_context.hpp Examining ./rclcpp/include/rclcpp/parameter.hpp Examining ./rclcpp/include/rclcpp/waitable.hpp Examining ./rclcpp/include/rclcpp/node.hpp Examining ./rclcpp/include/rclcpp/parameter_events_filter.hpp Examining ./rclcpp/include/rclcpp/executors.hpp Examining ./rclcpp/include/rclcpp/duration.hpp Examining ./rclcpp/include/rclcpp/any_service_callback.hpp Examining ./rclcpp/include/rclcpp/strategies/message_pool_memory_strategy.hpp Examining ./rclcpp/include/rclcpp/strategies/allocator_memory_strategy.hpp Examining ./rclcpp/include/rclcpp/any_subscription_callback.hpp Examining ./rclcpp/include/rclcpp/qos_event.hpp Examining ./rclcpp/include/rclcpp/detail/resolve_use_intra_process.hpp Examining ./rclcpp/include/rclcpp/detail/rmw_implementation_specific_publisher_payload.hpp Examining ./rclcpp/include/rclcpp/detail/rmw_implementation_specific_subscription_payload.hpp Examining ./rclcpp/include/rclcpp/detail/rmw_implementation_specific_payload.hpp Examining ./rclcpp/include/rclcpp/detail/resolve_intra_process_buffer_type.hpp Examining ./rclcpp/include/rclcpp/intra_process_buffer_type.hpp Examining ./rclcpp/include/rclcpp/context.hpp Examining ./rclcpp/include/rclcpp/exceptions.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_base.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_timers_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_services.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_clock_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_services_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_topics.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_time_source_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_graph.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_time_source.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_base_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/get_node_topics_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_graph_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_waitables_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/get_node_base_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/get_node_timers_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_parameters.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_parameters_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_logging_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_topics_interface.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_logging.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_timers.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_clock.hpp Examining ./rclcpp/include/rclcpp/node_interfaces/node_waitables.hpp Examining ./rclcpp/include/rclcpp/experimental/intra_process_manager.hpp Examining ./rclcpp/include/rclcpp/experimental/create_intra_process_buffer.hpp Examining ./rclcpp/include/rclcpp/experimental/subscription_intra_process_base.hpp Examining ./rclcpp/include/rclcpp/experimental/subscription_intra_process.hpp Examining ./rclcpp/include/rclcpp/experimental/buffers/ring_buffer_implementation.hpp Examining ./rclcpp/include/rclcpp/experimental/buffers/buffer_implementation_base.hpp Examining ./rclcpp/include/rclcpp/experimental/buffers/intra_process_buffer.hpp Examining ./rclcpp/include/rclcpp/publisher.hpp Examining ./rclcpp/include/rclcpp/intra_process_setting.hpp Examining ./rclcpp/include/rclcpp/create_subscription.hpp Examining ./rclcpp/include/rclcpp/init_options.hpp Examining ./rclcpp/include/rclcpp/node_options.hpp Examining ./rclcpp/include/rclcpp/graph_listener.hpp Examining ./rclcpp/include/rclcpp/memory_strategy.hpp Examining ./rclcpp/include/rclcpp/publisher_factory.hpp Examining ./rclcpp/include/rclcpp/parameter_value.hpp Examining ./rclcpp/include/rclcpp/subscription_base.hpp Examining ./rclcpp/include/rclcpp/time_source.hpp Examining ./rclcpp/include/rclcpp/parameter_map.hpp Examining ./rclcpp/include/rclcpp/visibility_control.hpp Examining ./rclcpp/include/rclcpp/qos.hpp Examining ./rclcpp/include/rclcpp/clock.hpp Examining ./rclcpp/include/rclcpp/subscription_traits.hpp Examining ./rclcpp/include/rclcpp/subscription_factory.hpp Examining ./rclcpp/include/rclcpp/create_service.hpp Examining ./rclcpp/include/rclcpp/memory_strategies.hpp Examining ./rclcpp/include/rclcpp/event.hpp Examining ./rclcpp/include/rclcpp/subscription_options.hpp Examining ./rclcpp/include/rclcpp/message_memory_strategy.hpp Examining ./rclcpp/include/rclcpp/create_timer.hpp Examining ./rclcpp/include/rclcpp/timer.hpp Examining ./rclcpp/include/rclcpp/scope_exit.hpp Examining ./rclcpp/include/rclcpp/rate.hpp Examining ./rclcpp/include/rclcpp/logger.hpp Examining ./rclcpp/include/rclcpp/allocator/allocator_common.hpp Examining ./rclcpp/include/rclcpp/allocator/allocator_deleter.hpp Examining ./rclcpp/include/rclcpp/create_client.hpp Examining ./rclcpp/include/rclcpp/callback_group.hpp Examining ./rclcpp/include/rclcpp/service.hpp Examining ./rclcpp/include/rclcpp/loaned_message.hpp Examining ./rclcpp/include/rclcpp/subscription.hpp Examining ./rclcpp/include/rclcpp/rclcpp.hpp Examining ./rclcpp/include/rclcpp/utilities.hpp Examining ./rclcpp/include/rclcpp/executors/single_threaded_executor.hpp Examining ./rclcpp/include/rclcpp/executors/multi_threaded_executor.hpp Examining ./rclcpp/include/rclcpp/publisher_base.hpp Examining ./rclcpp/include/rclcpp/publisher_options.hpp Examining ./rclcpp/include/rclcpp/type_support_decl.hpp Examining ./rclcpp/include/rclcpp/client.hpp Examining ./rclcpp/src/rclcpp/subscription_base.cpp Examining ./rclcpp/src/rclcpp/parameter_value.cpp Examining ./rclcpp/src/rclcpp/contexts/default_context.cpp Examining ./rclcpp/src/rclcpp/signal_handler.hpp Examining ./rclcpp/src/rclcpp/exceptions.cpp Examining ./rclcpp/src/rclcpp/parameter_client.cpp Examining ./rclcpp/src/rclcpp/parameter_events_filter.cpp Examining ./rclcpp/src/rclcpp/parameter_map.cpp Examining ./rclcpp/src/rclcpp/init_options.cpp Examining ./rclcpp/src/rclcpp/any_executable.cpp Examining ./rclcpp/src/rclcpp/detail/rmw_implementation_specific_payload.cpp Examining ./rclcpp/src/rclcpp/detail/rmw_implementation_specific_publisher_payload.cpp Examining ./rclcpp/src/rclcpp/detail/rmw_implementation_specific_subscription_payload.cpp Examining ./rclcpp/src/rclcpp/parameter_service_names.hpp Examining ./rclcpp/src/rclcpp/time_source.cpp Examining ./rclcpp/src/rclcpp/expand_topic_or_service_name.cpp Examining ./rclcpp/src/rclcpp/memory_strategies.cpp Examining ./rclcpp/src/rclcpp/node_interfaces/node_base.cpp Examining ./rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp Examining ./rclcpp/src/rclcpp/node_interfaces/node_topics.cpp Examining ./rclcpp/src/rclcpp/node_interfaces/node_time_source.cpp Examining ./rclcpp/src/rclcpp/node_interfaces/node_clock.cpp Examining ./rclcpp/src/rclcpp/node_interfaces/node_graph.cpp Examining ./rclcpp/src/rclcpp/node_interfaces/node_logging.cpp Examining ./rclcpp/src/rclcpp/node_interfaces/node_timers.cpp Examining ./rclcpp/src/rclcpp/node_interfaces/node_waitables.cpp Examining ./rclcpp/src/rclcpp/node_interfaces/node_services.cpp Examining ./rclcpp/src/rclcpp/clock.cpp Examining ./rclcpp/src/rclcpp/context.cpp Examining ./rclcpp/src/rclcpp/qos_event.cpp Examining ./rclcpp/src/rclcpp/duration.cpp Examining ./rclcpp/src/rclcpp/utilities.cpp Examining ./rclcpp/src/rclcpp/node_options.cpp Examining ./rclcpp/src/rclcpp/graph_listener.cpp Examining ./rclcpp/src/rclcpp/parameter_service.cpp Examining ./rclcpp/src/rclcpp/intra_process_manager.cpp Examining ./rclcpp/src/rclcpp/executors.cpp Examining ./rclcpp/src/rclcpp/node.cpp Examining ./rclcpp/src/rclcpp/qos.cpp Examining ./rclcpp/src/rclcpp/executor.cpp Examining ./rclcpp/src/rclcpp/memory_strategy.cpp Examining ./rclcpp/src/rclcpp/logger.cpp Examining ./rclcpp/src/rclcpp/waitable.cpp Examining ./rclcpp/src/rclcpp/callback_group.cpp Examining ./rclcpp/src/rclcpp/service.cpp Examining ./rclcpp/src/rclcpp/type_support.cpp Examining ./rclcpp/src/rclcpp/subscription_intra_process_base.cpp Examining ./rclcpp/src/rclcpp/parameter.cpp Examining ./rclcpp/src/rclcpp/client.cpp Examining ./rclcpp/src/rclcpp/event.cpp Examining ./rclcpp/src/rclcpp/signal_handler.cpp Examining ./rclcpp/src/rclcpp/timer.cpp Examining ./rclcpp/src/rclcpp/time.cpp Examining ./rclcpp/src/rclcpp/executors/multi_threaded_executor.cpp Examining ./rclcpp/src/rclcpp/executors/single_threaded_executor.cpp Examining ./rclcpp/src/rclcpp/publisher_base.cpp Examining ./rclcpp/test/test_subscription_traits.cpp Examining ./rclcpp/test/test_expand_topic_or_service_name.cpp Examining ./rclcpp/test/test_serialized_message_allocator.cpp Examining ./rclcpp/test/test_subscription.cpp Examining ./rclcpp/test/test_publisher_subscription_count_api.cpp Examining ./rclcpp/test/test_function_traits.cpp Examining ./rclcpp/test/test_find_weak_nodes.cpp Examining ./rclcpp/test/test_subscription_publisher_count_api.cpp Examining ./rclcpp/test/test_duration.cpp Examining ./rclcpp/test/test_time_source.cpp Examining ./rclcpp/test/test_node.cpp Examining ./rclcpp/test/node_interfaces/node_wrapper.hpp Examining ./rclcpp/test/node_interfaces/test_get_node_interfaces.cpp Examining ./rclcpp/test/node_interfaces/test_does_not_compile/get_node_topics_interface_const_ref_rclcpp_node.cpp Examining ./rclcpp/test/node_interfaces/test_does_not_compile/get_node_topics_interface_const_ptr_wrapped_node.cpp Examining ./rclcpp/test/node_interfaces/test_does_not_compile/get_node_topics_interface_const_ref_wrapped_node.cpp Examining ./rclcpp/test/node_interfaces/test_does_not_compile/get_node_topics_interface_const_ptr_rclcpp_node.cpp Examining ./rclcpp/test/test_node_global_args.cpp Examining ./rclcpp/test/test_parameter_client.cpp Examining ./rclcpp/test/test_loaned_message.cpp Examining ./rclcpp/test/test_logger.cpp Examining ./rclcpp/test/test_parameter_map.cpp Examining ./rclcpp/test/test_timer.cpp Examining ./rclcpp/test/test_create_timer.cpp Examining ./rclcpp/test/test_executor.cpp Examining ./rclcpp/test/test_ring_buffer_implementation.cpp Examining ./rclcpp/test/test_externally_defined_services.cpp Examining ./rclcpp/test/test_logging.cpp Examining ./rclcpp/test/test_time.cpp Examining ./rclcpp/test/test_parameter.cpp Examining ./rclcpp/test/test_node_options.cpp Examining ./rclcpp/test/test_intra_process_buffer.cpp Examining ./rclcpp/test/test_service.cpp Examining ./rclcpp/test/test_utilities.cpp Examining ./rclcpp/test/test_init.cpp Examining ./rclcpp/test/test_rate.cpp Examining ./rclcpp/test/test_parameter_events_filter.cpp Examining ./rclcpp/test/test_intra_process_manager.cpp Examining ./rclcpp/test/executors/test_multi_threaded_executor.cpp Examining ./rclcpp/test/test_client.cpp Examining ./rclcpp/test/test_publisher.cpp FINAL RESULTS: ./rclcpp/test/test_logging.cpp:64: [4] (format) vsnprintf: If format strings can be influenced by an attacker, they can be exploited, and note that sprintf variations do not always \0-terminate (CWE-134). Use a constant for the format specification. ./rclcpp/src/rclcpp/node_options.cpp:319: [3] (buffer) getenv: Environment variables are untrustable input if they can be set by an attacker. They can have any content and length, and the same variable can be set more than once (CWE-807, CWE-20). Check environment variables carefully before using them. ./rclcpp/src/rclcpp/node_options.cpp:102: [2] (buffer) char: Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120). Perform bounds checking, use functions that limit length, or ensure that the size is larger than the maximum possible length. ./rclcpp/src/rclcpp/signal_handler.cpp:199: [2] (buffer) char: Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120). Perform bounds checking, use functions that limit length, or ensure that the size is larger than the maximum possible length. ./rclcpp/test/test_logging.cpp:63: [2] (buffer) char: Statically-sized arrays can be improperly restricted, leading to potential overflows or other issues (CWE-119!/CWE-120). Perform bounds checking, use functions that limit length, or ensure that the size is larger than the maximum possible length. ./rclcpp/src/rclcpp/signal_handler.cpp:172: [1] (buffer) strncpy: Easily used incorrectly; doesn't always \0-terminate or check for invalid pointers [MS-banned] (CWE-120). ./rclcpp/test/test_parameter.cpp:472: [1] (buffer) mismatch: Function does not check the second iterator for over-read conditions (CWE-126). This function is often discouraged by most C++ coding standards in favor of its safer alternatives provided since C++14. Consider using a form of this function that checks the second iterator before potentially overflowing it. ./rclcpp/test/test_parameter.cpp:477: [1] (buffer) mismatch: Function does not check the second iterator for over-read conditions (CWE-126). This function is often discouraged by most C++ coding standards in favor of its safer alternatives provided since C++14. Consider using a form of this function that checks the second iterator before potentially overflowing it. ./rclcpp/test/test_parameter.cpp:482: [1] (buffer) mismatch: Function does not check the second iterator for over-read conditions (CWE-126). This function is often discouraged by most C++ coding standards in favor of its safer alternatives provided since C++14. Consider using a form of this function that checks the second iterator before potentially overflowing it. ./rclcpp/test/test_parameter.cpp:506: [1] (buffer) mismatch: Function does not check the second iterator for over-read conditions (CWE-126). This function is often discouraged by most C++ coding standards in favor of its safer alternatives provided since C++14. Consider using a form of this function that checks the second iterator before potentially overflowing it. ./rclcpp/test/test_parameter.cpp:518: [1] (buffer) mismatch: Function does not check the second iterator for over-read conditions (CWE-126). This function is often discouraged by most C++ coding standards in favor of its safer alternatives provided since C++14. Consider using a form of this function that checks the second iterator before potentially overflowing it. ./rclcpp/test/test_parameter.cpp:523: [1] (buffer) mismatch: Function does not check the second iterator for over-read conditions (CWE-126). This function is often discouraged by most C++ coding standards in favor of its safer alternatives provided since C++14. Consider using a form of this function that checks the second iterator before potentially overflowing it. ./rclcpp/test/test_parameter.cpp:605: [1] (buffer) mismatch: Function does not check the second iterator for over-read conditions (CWE-126). This function is often discouraged by most C++ coding standards in favor of its safer alternatives provided since C++14. Consider using a form of this function that checks the second iterator before potentially overflowing it. ./rclcpp/test/test_parameter.cpp:610: [1] (buffer) mismatch: Function does not check the second iterator for over-read conditions (CWE-126). This function is often discouraged by most C++ coding standards in favor of its safer alternatives provided since C++14. Consider using a form of this function that checks the second iterator before potentially overflowing it. ./rclcpp/test/test_parameter.cpp:615: [1] (buffer) mismatch: Function does not check the second iterator for over-read conditions (CWE-126). This function is often discouraged by most C++ coding standards in favor of its safer alternatives provided since C++14. Consider using a form of this function that checks the second iterator before potentially overflowing it. ./rclcpp/test/test_parameter.cpp:639: [1] (buffer) mismatch: Function does not check the second iterator for over-read conditions (CWE-126). This function is often discouraged by most C++ coding standards in favor of its safer alternatives provided since C++14. Consider using a form of this function that checks the second iterator before potentially overflowing it. ./rclcpp/test/test_parameter.cpp:651: [1] (buffer) mismatch: Function does not check the second iterator for over-read conditions (CWE-126). This function is often discouraged by most C++ coding standards in favor of its safer alternatives provided since C++14. Consider using a form of this function that checks the second iterator before potentially overflowing it. ./rclcpp/test/test_parameter.cpp:656: [1] (buffer) mismatch: Function does not check the second iterator for over-read conditions (CWE-126). This function is often discouraged by most C++ coding standards in favor of its safer alternatives provided since C++14. Consider using a form of this function that checks the second iterator before potentially overflowing it. ANALYSIS SUMMARY: Hits = 18 Lines analyzed = 45121 in approximately 1.31 seconds (34388 lines/second) Physical Source Lines of Code (SLOC) = 30880 Hits@level = [0] 4 [1] 13 [2] 3 [3] 1 [4] 1 [5] 0 Hits@level+ = [0+] 22 [1+] 18 [2+] 5 [3+] 2 [4+] 1 [5+] 0 Hits/KSLOC@level+ = [0+] 0.712435 [1+] 0.582902 [2+] 0.161917 [3+] 0.0647668 [4+] 0.0323834 [5+] 0 Dot directories skipped = 2 (--followdotdir overrides) Minimum risk level = 1 Not every hit is necessarily a security vulnerability. There may be other security vulnerabilities; review your code! See 'Secure Programming HOWTO' (https://dwheeler.com/secure-programs) for more information.

flawfinder represents the severity of its reports with a number that goes from [0] to [5]. This maps rather nice (IMHO) to the Levels proposed so far by @wjwwood and others (in a reverse manner). In other words a proposal here would be that package quality Level 1 of ROS 2 requires that the output of flawfinder shows no flaw with a severity above [4]. Similarly, package quality Level 2 of ROS 2 requires that the output of flawfinder shows no flaw with a severity above [3] and so on.

use of RATS for quality qualification:

RATS stands for rough auditing tool for security. In this case, not intended for security but for quality, RATS can (similarly to other tools discussed above) add value for quality qualification. The rationale for introducing RATS in here comes from the fact that quality is often viewed as a precondition to safety, and safety is connected to security in a direct manner. Refer to ISO/IEC TS 17961:2013 which indicates that "in practice, security-critical and safety-critical code have the same requirements". The output of RATS over rclcpp is below:

RATS output of rclcpp ROS 2 package

root@robot:/opt/eloquent_ws/src/ros2/rclcpp# rats . Entries in perl database: 33 Entries in ruby database: 46 Entries in python database: 62 Entries in c database: 334 Entries in php database: 55 Analyzing ./rclcpp_lifecycle/src/state.cpp Analyzing ./rclcpp_lifecycle/src/node_interfaces/lifecycle_node_interface.cpp Analyzing ./rclcpp_lifecycle/src/lifecycle_node.cpp Analyzing ./rclcpp_lifecycle/src/transition.cpp Analyzing ./rclcpp_lifecycle/test/test_state_wrapper.cpp Analyzing ./rclcpp_lifecycle/test/test_callback_exceptions.cpp Analyzing ./rclcpp_lifecycle/test/test_state_machine_info.cpp Analyzing ./rclcpp_lifecycle/test/test_lifecycle_node.cpp Analyzing ./rclcpp_lifecycle/test/test_register_custom_callbacks.cpp Analyzing ./rclcpp_lifecycle/test/test_transition_wrapper.cpp Analyzing ./rclcpp_action/src/qos.cpp Analyzing ./rclcpp_action/src/server.cpp Analyzing ./rclcpp_action/src/server_goal_handle.cpp Analyzing ./rclcpp_action/src/client.cpp Analyzing ./rclcpp_action/src/types.cpp Analyzing ./rclcpp_action/test/test_traits.cpp Analyzing ./rclcpp_action/test/test_server.cpp Analyzing ./rclcpp_action/test/test_client.cpp Analyzing ./rclcpp_components/src/component_container.cpp Analyzing ./rclcpp_components/src/component_container_mt.cpp Analyzing ./rclcpp_components/src/component_manager.cpp Analyzing ./rclcpp_components/test/test_component_manager.cpp Analyzing ./rclcpp_components/test/test_component_manager_api.cpp Analyzing ./rclcpp_components/test/components/test_component.cpp Analyzing ./rclcpp/src/rclcpp/subscription_base.cpp Analyzing ./rclcpp/src/rclcpp/parameter_value.cpp Analyzing ./rclcpp/src/rclcpp/contexts/default_context.cpp Analyzing ./rclcpp/src/rclcpp/exceptions.cpp Analyzing ./rclcpp/src/rclcpp/parameter_client.cpp Analyzing ./rclcpp/src/rclcpp/parameter_events_filter.cpp Analyzing ./rclcpp/src/rclcpp/parameter_map.cpp Analyzing ./rclcpp/src/rclcpp/init_options.cpp Analyzing ./rclcpp/src/rclcpp/any_executable.cpp Analyzing ./rclcpp/src/rclcpp/detail/rmw_implementation_specific_payload.cpp Analyzing ./rclcpp/src/rclcpp/detail/rmw_implementation_specific_publisher_payload.cpp Analyzing ./rclcpp/src/rclcpp/detail/rmw_implementation_specific_subscription_payload.cpp Analyzing ./rclcpp/src/rclcpp/time_source.cpp Analyzing ./rclcpp/src/rclcpp/expand_topic_or_service_name.cpp Analyzing ./rclcpp/src/rclcpp/memory_strategies.cpp Analyzing ./rclcpp/src/rclcpp/node_interfaces/node_base.cpp Analyzing ./rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp Analyzing ./rclcpp/src/rclcpp/node_interfaces/node_topics.cpp Analyzing ./rclcpp/src/rclcpp/node_interfaces/node_time_source.cpp Analyzing ./rclcpp/src/rclcpp/node_interfaces/node_clock.cpp Analyzing ./rclcpp/src/rclcpp/node_interfaces/node_graph.cpp Analyzing ./rclcpp/src/rclcpp/node_interfaces/node_logging.cpp Analyzing ./rclcpp/src/rclcpp/node_interfaces/node_timers.cpp Analyzing ./rclcpp/src/rclcpp/node_interfaces/node_waitables.cpp Analyzing ./rclcpp/src/rclcpp/node_interfaces/node_services.cpp Analyzing ./rclcpp/src/rclcpp/clock.cpp Analyzing ./rclcpp/src/rclcpp/context.cpp Analyzing ./rclcpp/src/rclcpp/qos_event.cpp Analyzing ./rclcpp/src/rclcpp/duration.cpp Analyzing ./rclcpp/src/rclcpp/utilities.cpp Analyzing ./rclcpp/src/rclcpp/node_options.cpp Analyzing ./rclcpp/src/rclcpp/graph_listener.cpp Analyzing ./rclcpp/src/rclcpp/parameter_service.cpp Analyzing ./rclcpp/src/rclcpp/intra_process_manager.cpp Analyzing ./rclcpp/src/rclcpp/executors.cpp Analyzing ./rclcpp/src/rclcpp/node.cpp Analyzing ./rclcpp/src/rclcpp/qos.cpp Analyzing ./rclcpp/src/rclcpp/executor.cpp Analyzing ./rclcpp/src/rclcpp/memory_strategy.cpp Analyzing ./rclcpp/src/rclcpp/logger.cpp Analyzing ./rclcpp/src/rclcpp/waitable.cpp Analyzing ./rclcpp/src/rclcpp/callback_group.cpp Analyzing ./rclcpp/src/rclcpp/service.cpp Analyzing ./rclcpp/src/rclcpp/type_support.cpp Analyzing ./rclcpp/src/rclcpp/subscription_intra_process_base.cpp Analyzing ./rclcpp/src/rclcpp/parameter.cpp Analyzing ./rclcpp/src/rclcpp/client.cpp Analyzing ./rclcpp/src/rclcpp/event.cpp Analyzing ./rclcpp/src/rclcpp/signal_handler.cpp Analyzing ./rclcpp/src/rclcpp/timer.cpp Analyzing ./rclcpp/src/rclcpp/time.cpp Analyzing ./rclcpp/src/rclcpp/executors/multi_threaded_executor.cpp Analyzing ./rclcpp/src/rclcpp/executors/single_threaded_executor.cpp Analyzing ./rclcpp/src/rclcpp/publisher_base.cpp Analyzing ./rclcpp/test/test_subscription_traits.cpp Analyzing ./rclcpp/test/test_expand_topic_or_service_name.cpp Analyzing ./rclcpp/test/test_serialized_message_allocator.cpp Analyzing ./rclcpp/test/test_subscription.cpp Analyzing ./rclcpp/test/test_publisher_subscription_count_api.cpp Analyzing ./rclcpp/test/test_function_traits.cpp Analyzing ./rclcpp/test/test_find_weak_nodes.cpp Analyzing ./rclcpp/test/test_subscription_publisher_count_api.cpp Analyzing ./rclcpp/test/test_duration.cpp Analyzing ./rclcpp/test/test_time_source.cpp Analyzing ./rclcpp/test/test_node.cpp Analyzing ./rclcpp/test/node_interfaces/test_get_node_interfaces.cpp Analyzing ./rclcpp/test/node_interfaces/test_does_not_compile/get_node_topics_interface_const_ref_rclcpp_node.cpp Analyzing ./rclcpp/test/node_interfaces/test_does_not_compile/get_node_topics_interface_const_ptr_wrapped_node.cpp Analyzing ./rclcpp/test/node_interfaces/test_does_not_compile/get_node_topics_interface_const_ref_wrapped_node.cpp Analyzing ./rclcpp/test/node_interfaces/test_does_not_compile/get_node_topics_interface_const_ptr_rclcpp_node.cpp Analyzing ./rclcpp/test/test_node_global_args.cpp Analyzing ./rclcpp/test/test_parameter_client.cpp Analyzing ./rclcpp/test/test_loaned_message.cpp Analyzing ./rclcpp/test/test_logger.cpp Analyzing ./rclcpp/test/test_parameter_map.cpp Analyzing ./rclcpp/test/test_timer.cpp Analyzing ./rclcpp/test/test_create_timer.cpp Analyzing ./rclcpp/test/test_executor.cpp Analyzing ./rclcpp/test/test_ring_buffer_implementation.cpp Analyzing ./rclcpp/test/test_externally_defined_services.cpp Analyzing ./rclcpp/test/test_logging.cpp Analyzing ./rclcpp/test/test_time.cpp Analyzing ./rclcpp/test/test_parameter.cpp Analyzing ./rclcpp/test/test_node_options.cpp Analyzing ./rclcpp/test/test_intra_process_buffer.cpp Analyzing ./rclcpp/test/test_service.cpp Analyzing ./rclcpp/test/test_utilities.cpp Analyzing ./rclcpp/test/test_init.cpp Analyzing ./rclcpp/test/test_rate.cpp Analyzing ./rclcpp/test/test_parameter_events_filter.cpp Analyzing ./rclcpp/test/test_intra_process_manager.cpp Analyzing ./rclcpp/test/executors/test_multi_threaded_executor.cpp Analyzing ./rclcpp/test/test_client.cpp Analyzing ./rclcpp/test/test_publisher.cpp ./rclcpp/src/rclcpp/node_options.cpp:319: High: getenv Environment variables are highly untrustable input. They may be of any length, and contain any data. Do not make any assumptions regarding content or length. If at all possible avoid using them, and if it is necessary, sanitize them and truncate them to a reasonable length. ./rclcpp/src/rclcpp/signal_handler.cpp:199: High: fixed size local buffer ./rclcpp/test/test_parameter_map.cpp:36: High: fixed size local buffer ./rclcpp/test/test_parameter_map.cpp:223: High: fixed size local buffer ./rclcpp/test/test_logging.cpp:63: High: fixed size local buffer Extra care should be taken to ensure that character arrays that are allocated on the stack are used safely. They are prime targets for buffer overflow attacks. ./rclcpp/src/rclcpp/signal_handler.cpp:195: Medium: signal When setting signal handlers, do not use the same function to handle multiple signals. There exists the possibility a race condition will result if 2 or more different signals are sent to the process at nearly the same time. Also, when writing signal handlers, it is best to do as little as possible in them. The best strategy is to use the signal handler to set a flag, that another part of the program tests and performs the appropriate action(s) when it is set. See also: http://razor.bindview.com/publish/papers/signals.txt Total lines analyzed: 25004 Total time 0.082441 seconds 303295 lines per second

In this case, RATS outputs signals which can be interpreted and mapped to quality. One way to go on this is that packages of quality Level 2 or above should not have "High" signals in RATS.

From our research, the use of these tools for static analysis is a common practice. There are other proprietary tools but all of the tools listed above are open source and open to use (each with its own license though). Note that the proposals above come together in a "disconnected" manner. They do not conform a common formal testing framework (at least not at this point). We could use some of them, all or none at all. From our perspective, we believe that qualifying quality via these testing tools (or maybe others) will add relevant value to the community. Not just for QA but also for security purposes (which as justified above is connected to quality and safety).

On the implementation side (it always comes down to that I guess), provided there's enough support and blessing from the people participating and Open Robotics' side, Alias could probably commit some effort to push the use of these tools to the ROS 2 build farm.

For compleness and complementary to the proposals above, MISRA C and other standards for software do provide a series of rules in a "common framework". This would be a larger undertake and we don't have the bandwidth to do it all (we could collaborate though!) but using codequality or statick together with custom MISRA C rules would add relevant value.

@vmayoral this is a great resource!

However, I don't think it belongs in the REP. Instead I'd like to see that in the recommendation for what to use, either as part of ament_lint_common or the ROS developer guide.

Also, any package may use more than what is in ament_lint_common and mention it in their "quality declaration" as described in this REP.

@wjwwood I see your point and understand we want to limit the scope to "Quality Assurance" in here however, my recent readings made me more convinced of the inherent connection between quality, security and integrity. Tools like the ones proposed above can help achieve an intuition of compliance to these levels (though @gavanderhoorn speaks right, it will not ensure quality compliance).

I'm ok bringing those proposals above to the ROS developer guide or to https://github.com/ament/ament_lint however, for that to make sense in this context, we'd need to add such restrictions somehow to this REP, shouldn't we? In other words, some quality levels in here should demand certain responses from those (or other) tools. I don't see that reflected in the REP currently and this concerns me at the moment.

Please see the following text I recently shared in a security discussion that I'm producing as part of some internal work we're conducing to further improve the devops cycle in robotics:

Quality(Quality Assurance or QA for short) and Security are often misunderstood when it comes to software. Ivers argues ¹ that quality "essentially means that the software will execute according to its design and purpose" while "security means that the software will not put data or computing systems at risk of unauthorized access". Within ¹ one relevant question that arises is whether the quality problems are also security issues or vice versa. Ivers indicates that quality bugs can turn into security ones, provided they’re exploitable, and addresses the question by remarking that quality and security are critical components to a broader notion: software integrity.

Footnotes

@misc{ivers_2017, title={Security vs. Quality: What's the Difference?}, url={https://www.securityweek.com/security-vs-quality-what’s-difference}, journal={SecurityWeek}, author={Ivers, Jim}, year={2017}, month={Mar}} ↩ ↩²

gavanderhoorn

Good initiative.

Some high-level comments/suggestions:

number the requirements. That would make it easier to refer to them and would also make the document DRY-er as descriptions of quality levels could then be defined as (sub)sets of numbered requirements. It would also make it easier to see the difference between the various levels.
as this seems to be a document proposing quality levels for all ROS (2?) packages, perhaps the requirements (or intended quality levels) for "ROS 2 core" packages should not be in this document. It seems strange to treat those as special in this REP (additionally: what is "core" changes, but is not defined here). If the requirements were numbered, a separate document could be used to list the requirements for core ROS 2 packages (ie: "REQ3.1 in REP-2004", similar to how some REPs claim compliance with other REPs currently (ie: 105 with 103)). If we really want it here in this document, perhaps move the ROS Core requirements to a separate section.
consider providing a mapping to Technology Readiness Levels (TRL). This would help tremendously with grounding this REP in vernacular that has been around for a long time and facilitates communicating these concepts to industry users as they are used to thinking in these terms. Sentences such as "packages which are needed for production systems" seem to imply this should be possible.

And I apologise for adding suggestions on some spelling/formatting aspects of the REP.

rep-2004.rst

Signed-off-by: William Woodall <william@osrfoundation.org>

wjwwood · 2019-12-19T00:56:36Z

number the requirements

+1, a great idea.

as this seems to be a document proposing quality levels for all ROS (2?) packages, perhaps the requirements (or intended quality levels) for "ROS 2 core" packages should not be in this document

That's the plan. The REP should be general, the [ROS Core] items will be removed and moved to the "ROS 2 Developer Guide" (https://index.ros.org/doc/ros2/Contributing/Developer-Guide/#package-quality-categories). It will be linked to from this REP as a "reference implementation" for meeting the requirements.

consider providing a mapping to Technology Readiness Levels (TRL).

I will look into that, thanks!

And I ap[p]ologise for adding suggestions on some spelling/formatting aspects of the REP.

They are appreciated. :)

wjwwood · 2019-12-19T01:00:35Z

I'll also just quickly cross-reference a suggested edit from @thomas-moulard about release policies and security policies, specifically responding to security vulnerability disclosures:

ros2/ros2_documentation#460 (comment)

I think both could be included in the document, but we need to finish the discussion and get some concrete recommendations for the language that would go in the REP.

itualami · 2019-12-20T09:38:18Z

Great initiative! These are some of my comments on the requirements:

"All pull requests will require at least one reviewer who did not author the pr (package may choose to increase this number)". Most communities I studied have a rule of a minimum 2-3 reviewers for each PR. One reviewer per PR seems very low to me and not within FOSS communities' standards. Especially for “Level 1”, I’d recommend a mandatory 3 reviewers approve the quality of the code and any subsequent changes to the package.
Other communities also enforce these software engineering (SE) principles:
a. Maintainability of the code.
b. Free of technical debt.
c. Adheres to best practices.

Obviously, these SE principles should be defined and tailored for ROS.

There is also the process of "claiming" a Quality Level (QL), which is defined in the REP by self-claiming. I suggest that a “committee” of reviewers is appointed to make a judgement on the QL of a package and these questions should be addressed: How a package maintainer can claim a particular QL? Who has the authority of assign these QL? There should be a review and approval for a QL before it can be claimed by the package maintainer. I strongly recommend badges for these QLs. The effect of badges on the behavior of FOSS contributors has been empirically studied and shows significant impact; it motivates contributors to strive for the best reward (i.e. badges).

Happy to discuss further and contribute.

Unemyr · 2019-12-31T01:13:15Z

I would like to propose something along the lines of below, and feel free to comment and/or add your suggestions and additions.

Level 4 - Industrial Grade and Sustained
High level of quality and processes in place for long term sustenance.

Acceptance criteria:

All acceptance criteria from level 3
Unit test code coverage at least 90% of total code base
Dynamic analysis (asan/tsan/msan) run on all the unit tests with no errors/warnings
(This is very nice, because running these tools on the unit tests suite itself ensures auditable code coverage also for the asan/tsan/msan tools run!)
Dynamic analysis (asan/tsan/msan) run on intended use case/tutorials of the package with no errors/warnings
Performance profiling conducted to ensure any major performance bottlenecks have been eliminated - i e the intent of this level of code is that it should also be production performance, not just production quality.

Level 3 - Mature and Sustained
Good level of quality and processes in place for long term sustenance.

Acceptance criteria:

All acceptance criteria from level 2
Continuous integration activated
Static analysis (LINT) with no errors/warnings of total code base
Unit test code coverage at least 70% of total code base
Identified ownership/maintainers of the module

Level 2 - Functional
At least common/basic set of features are fully available and proven available. Quality processes applied at this code level may be missing, or not yet meeting the acceptance criteria of higher levels.

Acceptance criteria:

Key features manually tested either in practice (if it required hardware devices to be used) or in simulation if its a software only solution.
Proper readme .md /documentation that articulates key functionality of the package, versions/platforms supported and how to install the solution
Quality badges to be setup for the package (that would then show that the package is not yet at the higher levels of quality assurance)
Simple tutorial .md that articulates how to use the package

Level 1 - Experimental
Work in progress, prototype code that may or may not work at full intended features or capabilities. Quality processes applied at this code level is not really relevant or assessed, as the feature set is not yet there. Basically an "at your own risk" package download.

Acceptance criteria:
None

My feeling with above would be, that anyone that is doing some development somewhere and is relatively serious about contributing something back, could easily generate a level 2 package. There is a little bit of a jump to reach level 3, because it takes effort to develop the unit test code coverage, and not all developers are familiar with the different way of developing code when you are generating good unit tests. Once you have already achieved level 3, the step to level 4 should not feel unsurpassable because you only need to complete the remaining part of code coverage, and then run the sanitizer tools on this run and a few extra steps, and you would be hitting level 4. So there could be a carrot to just go for it to reach level 4 if you already made all the effort to reach level 3. The biggest hurdle for many of these processes, for most developers, is just being familiar with using the tools.

wjwwood · 2020-01-03T23:42:17Z

Thanks for the feedback folks, I'll get back to this soon.

@Unemyr Please make concrete (maybe I mean "more granular") suggestions (ideally with pull requests or diffs). Your feedback, while appreciated, would essentially replace part of this new REP but isn't as comprehensive as the parts being replaced. This is very difficult to integrate and to comment on. Please instead suggest changes to the existing document, which several people have already put a lot of time into.

wjwwood · 2020-01-04T00:50:07Z

@Unemyr sorry, maybe what I meant instead of "concrete suggestions" was more granular. Same for you @itualami. For example, your first point, it would have been to make a line comment on that part of the document, so we can discuss each point in their own thread and resolve them as they are resolved. Sort of like what @gavanderhoorn did with his line comments.

I don't mean to detract from the feedback on points of process, but I think this approach would help us keep things organized. Thanks!

wjwwood · 2020-01-04T01:05:03Z

Other communities also enforce these software engineering (SE) principles:

Sure, I'll try to comment on each.

a. Maintainability of the code.

I don't know how to measure this, so I hesitate to put it in. One of the main goals of this is to encourage packages to describe their quality so that consumers can be informed about them, and so if something is not very tangible or measurable then it's hard to say what needs to go into the quality declaration. We could say you need to have a policy about this written down? But I don't see how it can be enforced and it strays into "write good software", and is therefore kind of subjective (i.e. how maintainable is maintainable enough? what makes it maintainable or not?).

This is definitely something I would encourage people to consider when taking changes, but I don't know how to capture it in this effort.

b. Free of technical debt.

This is a great ideal, but in my experience not practical if taken absolutely. All changes are by their nature incremental. And striving for a complete (or perfect) set of changes before taking them into the project can often be detrimental to the project.

Again, this strays into the "write good software" zone for me and so I'm hesitant to try and capture it here. However, I'm open to considering specific language which may be added (diff or pull request).

c. Adheres to best practices.

Obviously, these SE principles should be defined and tailored for ROS.

This is too vague for me personally. What are the best practices? How do we measure or enforce them?

How would you tailor them? The more specific the suggestion, the more likely we can take it into the document.

There is also the process of "claiming" a Quality Level (QL), which is defined in the REP by self-claiming. I suggest that a “committee” of reviewers is appointed to make a judgement on the QL of a package and these questions should be addressed: How a package maintainer can claim a particular QL? Who has the authority of assign these QL? There should be a review and approval for a QL before it can be claimed by the package maintainer. I strongly recommend badges for these QLs. The effect of badges on the behavior of FOSS contributors has been empirically studied and shows significant impact; it motivates contributors to strive for the best reward (i.e. badges).

For me personally, it is an anti-goal to require us to form a governing/certifying body. Instead, the goal is to encourage package maintainers to declare a level and justify it in their "quality declaration", not so that they may be given a sticker or achievement but so that they have to think about it, and so that it is documented in a way that consumers may be informed and make their own judgement.

I allow for (suggest) that we could setup curated lists of packages in different quality levels. Having such lists would address the concern for users which don't have time to investigate each package on their own. They would essentially be trusting the group that curated the list rather than spending time investigating it themselves.

I think that having badges for the curated list is a good idea, but I don't think this document has the power to commission a committee or setup the infrastructure for badges. I think those things can and will be done, but I don't think it belongs in this document and I don't think there just needs to be one list/committee/set of badges/etc.

rep-0133.rst

rosbuild · 2020-02-06T16:37:25Z

This pull request has been mentioned on ROS. There might be relevant details there:

https://discourse.ros.org/t/next-ros-quality-assurance-working-group-february-2020/12571/3

kyrofa

Thanks for this @wjwwood, I like the approach. I need to review it more in-depth, but I wanted to add some high-level comments. Note that we're discussing this in the security WG as well; it ties in nicely with the VDP work we're doing, and it seemingly makes sense to have some security criteria in here as well (e.g. responsiveness to disclosures, etc.). We'll get some more specifics soon.

rep-2004.rst

ros-discourse · 2020-02-10T16:55:54Z

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-control-abi-breakage-in-melodic-2020-02/12681/7

rep-2004.rst

wjwwood · 2020-02-19T23:57:56Z

Note that we're discussing this in the security WG as well; it ties in nicely with the VDP work we're doing, and it seemingly makes sense to have some security criteria in here as well (e.g. responsiveness to disclosures, etc.). We'll get some more specifics soon.

That would be great.

ros-discourse · 2020-02-20T16:00:20Z

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/introducing-ros2-sanitizer-report-and-analysis/9287/7

ros-discourse · 2020-02-21T00:21:27Z

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2020-02-20/12901/1

* REP 2004 extras Signed-off-by: maryaB-osr <marya@openrobotics.org> * possible chart structure and license question Signed-off-by: maryaB-osr <marya@openrobotics.org> * comment out chart Signed-off-by: maryaB-osr <marya@openrobotics.org> * restructure and edits Signed-off-by: maryaB-osr <marya@openrobotics.org> * fixed a header Signed-off-by: maryaB-osr <marya@openrobotics.org> * add chart Signed-off-by: maryaB-osr <marya@openrobotics.org> * q. level 3 testing in wrong order Signed-off-by: maryaB-osr <marya@openrobotics.org> * level 3 3.v.a mistake Signed-off-by: maryaB-osr <marya@openrobotics.org> * moved chart to be under level 1 makes it easier to read the numbers Signed-off-by: maryaB-osr <marya@openrobotics.org> * wjwwood review Signed-off-by: maryaB-osr <marya@openrobotics.org> * edits / chart check Signed-off-by: maryaB-osr <marya@openrobotics.org> * fix python incompatibility Signed-off-by: maryaB-osr <marya@openrobotics.org>

gavanderhoorn · 2020-04-22T15:15:52Z

Sorry to bring this up so late, but would it be an idea to explicitly state the REP follows RFC-2119 when it comes to the use of should, must, must not, etc? It's a fairly well established RFC for these kinds of things -- and used outside of internet standards as well.

Something along the lines of:

The key words must, must not, required, shall, shall not, should, should not, recommended, may, and optional in this document are to be interpreted as defined in RFC2119.

If we agree, I could contribute a PR.

rep-2004.rst

ros-discourse · 2020-04-24T22:21:28Z

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/may-quality-assurance-working-group-meeting/13812/1

rep-2004.rst

Co-Authored-By: Alejandro Hernández Cordero <ahcorde@gmail.com>

wjwwood · 2020-04-27T17:28:22Z

If we agree, I could contribute a PR.

Sounds reasonable to me, and we have lots of 👍 reactions to your comment.

Co-Authored-By: Marya Belanger <marya@openrobotics.org>

Co-Authored-By: Kyle Fazzari <github@status.e4ward.com>

wjwwood · 2020-04-28T18:04:19Z

rep-2004.rst

+REP: 2004
+Title: Package Quality Categories
+Author: William Woodall <william@openrobotics.org>
+Status: Active


Please vote in reply here with +1 or -1 to accept this REP or not, respectively. Thanks!

👍 This is a well thought-through process and has evolved impressively over the last few months. I think it will lead to maintainers striving for higher quality of their own packages, and maybe it will get some people to actually commit to releasing non-beta versions of their packages 😝

I think the copyright policy needs work but other than that 👍

Ok, I think I've addressed the copyright point for now, we can iterate on it further if needed. We have some +1's here and at least no major -1's, so I'm going to merge this (extremely long running) pull request and redirect everything else to new issues.

Thanks everyone!

rep-2004.rst

rotu · 2020-04-29T01:19:51Z

rep-2004.rst

+
+.. _Testing:
+
+4. **Testing:**


What is the testing policy for things like threadsafety?

I don't think we need anything specific to that, to be honest. The documentation should probably state what the thread-safety status of the methods are, and the tests should aim to check that documented behavior.

I think something like this is out of the scope of this REP because I think (i.e. my opinion) it strays into "write good software" which always tends to be subjective and hard to completely quantify in my experience. Like how do you measure when enough thread-safety testing has been done, does it need to proven somehow (logical proof), is it empirical testing? I don't know. Also, why just thread-safety, why not memory allocations, or blocking behavior, or some other aspect of the software.

If you have some specific in mind, please suggest what you think a reasonable entry here would be and maybe we can agree on something generic and useful enough to include here.

It may also be something good to have in our developer guide or in a contributing or pull request review check list, depending on what you were thinking.

I think something like this is out of the scope of this REP because I think (i.e. my opinion) it strays into "write good software" which always tends to be subjective and hard to completely quantify in my experience.

I agree that this is REALLY hard to nail down in a prescriptive way, and quite possibly belongs elsewhere. It also likely doesn't belong under Testing unless and until the community has had more experience ensuring threadsafe code.

If you have some specific in mind, please suggest what you think a reasonable entry here would be and maybe we can agree on something generic and useful enough to include here.

Challenge accepted!

Platform Support:
ii. Should document assumptions of the runtime environment, including whether APIs may be used multithreaded, whether filesystem access is required, and what additional sensors or hardware are expected.

At the risk of loosing momentum on this point, I want to wrap up this first version of the REP. I will make a follow up issue for this and if you have time a pull request with what you've suggested would be easier to comment on (and make sure things like: it is considered or not for each quality level, and it is in the table of requirements, etc...).

maryaB-osr · 2020-04-29T07:27:07Z

@gavanderhoorn @wjwwood This might be pedantic, but if we're going to say

The key words must, must not, required, shall, shall not, should, should not, recommended, may, and optional in this document are to be interpreted as defined in RFC2119.

Then should we use a different symbol in the chart to represent "may" statements, since "may" corresponds to "optional" in RFC2119, but right now we have both "should" and "may" statements marked with ● ("recommended")? There are 5 requirements using "may" instead of "should"

If yes, how about this hollow circle ○ for "optional"?

gavanderhoorn · 2020-04-29T08:50:17Z

@gavanderhoorn @wjwwood This might be pedantic, but if we're going to say

The key words must, must not, required, shall, shall not, should, should not, recommended, may, and optional in this document are to be interpreted as defined in RFC2119.

Then should we use a different symbol in the chart to represent "may" statements, since "may" corresponds to "optional" in RFC2119, but right now we have both "should" and "may" statements marked with ● ("recommended")? There are 5 requirements using "may" instead of "should"

If yes, how about this hollow circle ○ for "optional"?

Hm. I wouldn't really have a preference. As long as we clearly differentiate between the different requirements. An open circle seems like it would suffice.

Signed-off-by: maryaB-osr <marya@openrobotics.org>

wjwwood · 2020-04-29T22:02:38Z

@maryaB-osr that sounds fine to me.

rep-2004.rst

+ "higher quality" distinction

Signed-off-by: William Woodall <william@osrfoundation.org>

ablasdel · 2020-05-05T18:37:28Z

Huzzah!

ros-discourse · 2020-05-07T15:28:49Z

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/may-quality-assurance-working-group-meeting/13812/6

initial commit of new rep-2004

e0cfb30

Signed-off-by: William Woodall <william@osrfoundation.org>

wjwwood self-assigned this Dec 17, 2019

wjwwood changed the title ~~initial commit of new rep-2004~~ REP-2004: Package Quality Categories Dec 17, 2019

update my (wjwwood's) email addresses, because the rep script only al…

7640cce

…lows one per person Signed-off-by: William Woodall <william@osrfoundation.org>

gavanderhoorn reviewed Dec 18, 2019

View reviewed changes

vmayoral reviewed Dec 18, 2019

View reviewed changes

gavanderhoorn reviewed Dec 18, 2019

View reviewed changes

fixup copyright/license statements

c2ba03b

Signed-off-by: William Woodall <william@osrfoundation.org>

mjcarroll reviewed Jan 22, 2020

View reviewed changes

rep-0133.rst Show resolved Hide resolved

This was referenced Jan 30, 2020

add quality declaration ros2/rcutils#202

Closed

initial draft of the quality declaration ros2/rcl#566

Closed

mjcarroll mentioned this pull request Feb 5, 2020

Remove MISRA compliance guide ros2/ros2_documentation#516

Merged

kyrofa reviewed Feb 8, 2020

View reviewed changes

rep-2004.rst Outdated Show resolved Hide resolved

mm318 reviewed Feb 18, 2020

View reviewed changes

rep-2004.rst Outdated Show resolved Hide resolved

maryaB-osr mentioned this pull request Feb 19, 2020

Quality Categories deserves its own file - not in Developer Guide ros2/ros2_documentation#531

Closed

thomas-moulard mentioned this pull request Mar 4, 2020

ROS 2 Foxy Fitzroy ros2/ros2#830

Closed

3 tasks

Blast545 mentioned this pull request Mar 5, 2020

Add QD draft for Boost exernal dependency ros2/ros2_documentation#551

Closed

Blast545 reviewed Apr 22, 2020

View reviewed changes

rep-2004.rst Outdated Show resolved Hide resolved

ahcorde reviewed Apr 27, 2020

View reviewed changes

rep-2004.rst Show resolved Hide resolved

Blast545 mentioned this pull request Apr 27, 2020

Add quality declaration for foonathan_memory eProsima/foonathan_memory_vendor#22

Merged

Update rep-2004.rst

eb5ba5a

Co-Authored-By: Alejandro Hernández Cordero <ahcorde@gmail.com>

wjwwood and others added 2 commits April 27, 2020 10:28

Update rep-2004.rst

d7a3591

Co-Authored-By: Marya Belanger <marya@openrobotics.org>

Update rep-2004.rst

4a3bbf6

Co-Authored-By: Kyle Fazzari <github@status.e4ward.com>

wjwwood commented Apr 28, 2020

View reviewed changes

brawner approved these changes Apr 29, 2020

View reviewed changes

rotu suggested changes Apr 29, 2020

View reviewed changes

maryaB-osr mentioned this pull request Apr 29, 2020

Adjusting 3.v.b requirement #253

Merged

Adjusting 3.v.b requirement (#253)

0282e1c

Signed-off-by: maryaB-osr <marya@openrobotics.org>

maryaB-osr reviewed Apr 30, 2020

View reviewed changes

rep-2004.rst Show resolved Hide resolved

maryaB-osr reviewed Apr 30, 2020

View reviewed changes

rep-2004.rst Outdated Show resolved Hide resolved

maryaB-osr reviewed Apr 30, 2020

View reviewed changes

rep-2004.rst Outdated Show resolved Hide resolved

maryaB-osr reviewed Apr 30, 2020

View reviewed changes

rep-2004.rst Outdated Show resolved Hide resolved

maryaB-osr and others added 2 commits April 30, 2020 16:12

Optional chart symbol

cfa44ee

+ "higher quality" distinction

address feedback about copyright statement requirements

99c2865

Signed-off-by: William Woodall <william@osrfoundation.org>

wjwwood merged commit e31d97b into master May 5, 2020

wjwwood deleted the rep-2004 branch May 5, 2020 18:37

Blast545 mentioned this pull request May 5, 2020

Add Quality Declaration level 4 ament_index_cpp ament/ament_index#53

Merged

ShotaAk mentioned this pull request May 13, 2020

Fixed a link to REP-2004 in QUALITY_DECLARATION.md ros2/rcutils#245

Merged

cardboardcode mentioned this pull request Jun 6, 2021

Adding ros-industrial EPD and EMD under Application Layer. fkromer/awesome-ros2#27

Merged

REP-2004: Package Quality Categories #218

REP-2004: Package Quality Categories #218

Conversation

wjwwood commented Dec 17, 2019 • edited Loading

Abstract

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

use of cppcheck for quality qualification:

use of flawfinder for quality qualification:

use of RATS for quality qualification:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Footnotes

gavanderhoorn left a comment

Choose a reason for hiding this comment

wjwwood commented Dec 19, 2019

wjwwood commented Dec 19, 2019

itualami commented Dec 20, 2019

Unemyr commented Dec 31, 2019

wjwwood commented Jan 3, 2020 • edited Loading

wjwwood commented Jan 4, 2020

wjwwood commented Jan 4, 2020

rosbuild commented Feb 6, 2020

kyrofa left a comment

Choose a reason for hiding this comment

ros-discourse commented Feb 10, 2020

wjwwood commented Feb 19, 2020

ros-discourse commented Feb 20, 2020

ros-discourse commented Feb 21, 2020

gavanderhoorn commented Apr 22, 2020 • edited Loading

ros-discourse commented Apr 24, 2020

wjwwood commented Apr 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rotu Apr 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rotu Apr 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maryaB-osr commented Apr 29, 2020

gavanderhoorn commented Apr 29, 2020

wjwwood commented Apr 29, 2020

ablasdel commented May 5, 2020

ros-discourse commented May 7, 2020

wjwwood commented Dec 17, 2019 •

edited

Loading

use of `cppcheck` for quality qualification:

use of `flawfinder` for quality qualification:

use of `RATS` for quality qualification:

wjwwood commented Jan 3, 2020 •

edited

Loading

gavanderhoorn commented Apr 22, 2020 •

edited

Loading

rotu Apr 29, 2020 •

edited

Loading

rotu Apr 29, 2020 •

edited

Loading