AMRNAV-6776 AMR diagnostics: Use CPU and RAM monitor from diagnostics

.github/workflows/lint.yaml

@@ -18,17 +18,23 @@ jobs:
             cppcheck,
             cpplint,
             flake8,
-            pep257,
+            # pep257, TODO: enable when we fixed
+            # Error: diagnostic_common_diagnostics/diagnostic_common_diagnostics/ntp_monitor.py:113 in public method `ntp_diag`: D417: Missing argument descriptions in the docstring (argument(s) st are missing descriptions in 'ntp_diag' docstring)
+            # using ros-rolling-ament-pep257 amd64 0.18.0-1noble.20240426.150718
             uncrustify,
             xmllint,
           ]
-    runs-on: ubuntu-22.04
+        include:
+          - distro: rolling
+            os: ubuntu-24.04
+    runs-on: ${{ matrix.os }}
     env:
       AMENT_CPPCHECK_ALLOW_SLOW_VERSIONS: 1
     steps:
       - uses: actions/checkout@v1
       - uses: ros-tooling/setup-ros@master
-      - run: sudo pip install pydocstyle==6.1.1  # downgrade to fix https://github.com/ament/ament_lint/pull/428
+        with:
+          required-ros-distributions: ${{ matrix.distro }}
       - uses: ros-tooling/action-ros-lint@master
         with:
           linter: ${{ matrix.linter }}

.github/workflows/test.yaml

@@ -14,28 +14,21 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        package:
-          [
+        package: [
             diagnostic_aggregator,
             diagnostic_common_diagnostics,
             diagnostic_updater,
             self_test,
           ]
-        distro: [humble, iron, rolling]
         include:
-          - distro: humble
-            os: ubuntu-22.04
-          - distro: iron
-            os: ubuntu-22.04
           - distro: rolling
-            os: ubuntu-22.04
-    runs-on: ${{ matrix.os }}
+            os: 24.04
+    runs-on: ubuntu-latest
+    container: ubuntu:${{ matrix.os }}
     steps:
       - uses: ros-tooling/setup-ros@master
-      - run: |
-          sudo pip install pydocstyle==6.1.1  # downgrade to fix https://github.com/ament/ament_lint/pull/428
-          sudo pip install pip --upgrade
-          sudo pip install pyopenssl --upgrade  # fix for AttributeError: module 'lib' has no attribute 'X509_V_FLAG_CB_ISSUER_CHECK'
+        with:
+          required-ros-distributions: ${{ matrix.distro }}
       - uses: ros-tooling/action-ros-ci@master
         with:
           target-ros2-distro: ${{ matrix.distro }}

diagnostic_aggregator/CHANGELOG.rst

@@ -2,6 +2,25 @@
 Changelog for package diagnostic_aggregator
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+4.3.1 (2024-07-30)
+------------------
+
+3.2.1 (2024-06-27)
+------------------
+* Add add_analyzer functionality (`#329 <https://github.com/ros/diagnostics/issues/329>`_)
+* Aggregator: publish diagnostics_toplevel_state immediately on every degradation (`#324 <https://github.com/ros/diagnostics/issues/324>`_)
+* Contributors: MartinCornelis2, Tim Clephas
+
+3.2.0 (2024-03-22)
+------------------
+* Avoid rolling up an ERROR state when empty GenericAnalyzer blocks are marked discard_stale, or when all of their items are STALE. (`#315 <https://github.com/ros/diagnostics/issues/315>`_)
+* formatting fixes from PR324 (`#327 <https://github.com/ros/diagnostics/issues/327>`_)
+* Debugging instability introduced by `#317 <https://github.com/ros/diagnostics/issues/317>`_  (`#323 <https://github.com/ros/diagnostics/issues/323>`_)
+* feat: publish top level msg when error is received (`#317 <https://github.com/ros/diagnostics/issues/317>`_)
+* Empty default aggregator base_path (`#305 <https://github.com/ros/diagnostics/issues/305>`_)
+* using defined state for stale (`#298 <https://github.com/ros/diagnostics/issues/298>`_)
+* Contributors: Andrew Symington, Christian Henkel, outrider-jhulas
+
 3.1.2 (2023-03-24)
 ------------------
 

diagnostic_aggregator/CMakeLists.txt

@@ -14,6 +14,7 @@ find_package(ament_cmake REQUIRED)
 find_package(diagnostic_msgs REQUIRED)
 find_package(pluginlib REQUIRED)
 find_package(rclcpp REQUIRED)
+find_package(rcl_interfaces REQUIRED)
 find_package(std_msgs REQUIRED)
 find_package(rclcpp_components REQUIRED)
 
@@ -72,6 +73,10 @@ target_link_libraries(aggregator_node
 
 rclcpp_components_register_nodes(${PROJECT_NAME} "diagnostic_aggregator::Aggregator")
 
+# Add analyzer
+add_executable(add_analyzer src/add_analyzer.cpp)
+ament_target_dependencies(add_analyzer rclcpp rcl_interfaces)
+
 # Testing macro
 if(BUILD_TESTING)
   find_package(ament_lint_auto REQUIRED)
@@ -82,6 +87,7 @@ if(BUILD_TESTING)
   find_package(launch_testing_ament_cmake REQUIRED)
 
   file(TO_CMAKE_PATH "${CMAKE_INSTALL_PREFIX}/lib/${PROJECT_NAME}/aggregator_node" AGGREGATOR_NODE)
+  file(TO_CMAKE_PATH "${CMAKE_INSTALL_PREFIX}/lib/${PROJECT_NAME}/add_analyzer" ADD_ANALYZER)
   file(TO_CMAKE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/test/test_listener.py" TEST_LISTENER)
   set(create_analyzers_tests
     "primitive_analyzers"
@@ -129,17 +135,48 @@ if(BUILD_TESTING)
     )
   endforeach()
 
+  set(add_analyzers_tests
+  "all_analyzers")
+
+  foreach(test_name ${add_analyzers_tests})
+    file(TO_CMAKE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/test/default.yaml" PARAMETER_FILE)
+    file(TO_CMAKE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/test/${test_name}.yaml" ADD_PARAMETER_FILE)
+    file(TO_CMAKE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/test/expected_output/add_${test_name}" EXPECTED_OUTPUT)
+
+    configure_file(
+      "test/add_analyzers.launch.py.in"
+      "test_add_${test_name}.launch.py"
+      @ONLY
+    )
+    add_launch_test(
+      "${CMAKE_CURRENT_BINARY_DIR}/test_add_${test_name}.launch.py"
+      TARGET "test_add_${test_name}"
+      TIMEOUT 30
+      ENV
+    )
+  endforeach()
+
   add_launch_test(
     test/test_critical_pub.py
     TIMEOUT 30
   )
+
+  ament_add_pytest_test(test_discard_behavior
+    "${CMAKE_CURRENT_SOURCE_DIR}/test/test_discard_behavior.py"
+    TIMEOUT 60
+  )
 endif()
 
 install(
   TARGETS aggregator_node
   DESTINATION lib/${PROJECT_NAME}
 )
 
+install(
+  TARGETS add_analyzer
+  DESTINATION lib/${PROJECT_NAME}
+)
+
 install(
   TARGETS ${PROJECT_NAME} ${ANALYZERS}
   EXPORT ${PROJECT_NAME}Targets
@@ -157,6 +194,7 @@ ament_python_install_package(${PROJECT_NAME})
 
 # Install Example
 set(ANALYZER_PARAMS_FILEPATH "${CMAKE_INSTALL_PREFIX}/share/${PROJECT_NAME}/example_analyzers.yaml")
+set(ADD_ANALYZER_PARAMS_FILEPATH "${CMAKE_INSTALL_PREFIX}/share/${PROJECT_NAME}/example_add_analyzers.yaml")
 configure_file(example/example.launch.py.in example.launch.py @ONLY)
 install( # launch descriptor
   FILES ${CMAKE_CURRENT_BINARY_DIR}/example.launch.py
@@ -167,7 +205,7 @@ install( # example publisher
   DESTINATION lib/${PROJECT_NAME}
 )
 install( # example aggregator configration
-  FILES example/example_analyzers.yaml
+  FILES example/example_analyzers.yaml example/example_add_analyzers.yaml
   DESTINATION share/${PROJECT_NAME}
 )
 

diagnostic_aggregator/README.md

@@ -14,7 +14,7 @@ The robot has two of each, one on each side.
 The robot also 4 camera sensors, one left and one right and one in the front and one in the back.
 These are all the available diagnostic sources:
 
-``` 
+```
 /arms/left/motor
 /arms/right/motor
 /legs/left/motor
@@ -119,6 +119,9 @@ Additional parameters depend on the type of the analyzer.
 Any diagnostic item that is not matched by any analyzer will be published by an "Other" analyzer.
 Items created by the "Other" analyzer will go stale after 5 seconds.
 
+The `critical` parameter makes the aggregator react immediately to a degradation in diagnostic state.
+This is useful if the toplevel state is parsed by a watchdog for example.
+
 ## Launching
 You can launch the `aggregator_node` like this (see [example.launch.py.in](example/example.launch.py.in)):
 ``` python
@@ -132,6 +135,33 @@ You can launch the `aggregator_node` like this (see [example.launch.py.in](examp
     ])
 ```
 
+You can add analyzers at runtime using the `add_analyzer` node like this (see [example.launch.py.in](example/example.launch.py.in)):
+```
+    add_analyzer = launch_ros.actions.Node(
+        package='diagnostic_aggregator',
+        executable='add_analyzer',
+        output='screen',
+        parameters=[add_analyzer_params_filepath])
+    return launch.LaunchDescription([
+        add_analyzer,
+    ])
+```
+This node updates the parameters of the `aggregator_node` by calling the service `/analyzers/set_parameters_atomically`.
+The `aggregator_node` will detect when a `parameter-event` has introduced new parameters to it.
+When this happens the `aggregator_node` will reload all analyzers based on its new set of parameters.
+Adding analyzers this way can be done at runtime and can be made conditional.
+
+In the example, `add_analyzer` will add an analyzer for diagnostics that are marked optional:
+``` yaml
+/**:
+  ros__parameters:
+    optional:
+      type: diagnostic_aggregator/GenericAnalyzer
+      path: Optional
+      startswith: [ '/optional' ]
+```
+This will move the `/optional/runtime/analyzer` diagnostic from the "Other" to  "Aggregation" where it will not go stale after 5 seconds and will be taken into account for the toplevel state.
+
 # Basic analyzers
 The `diagnostic_aggregator` package provides a few basic analyzers that you can use to aggregate your diagnostics.
 
@@ -186,4 +216,4 @@ This means that things that are ignored by the `IgnoreAnalyzer` will still be pu
 - `analyzers` (map, default: {}) - The analyzers that will be used to aggregate the diagnostics
 
 # Tutorials
-TODO: Port tutorials #contributions-welcome
+TODO: Port tutorials #contributions-welcome

diagnostic_aggregator/example/README.md

@@ -1,5 +1,7 @@
 # Aggregator Example
 
-This is a simple example to show the diagnostic_aggregator in action. It involves one python script producing dummy diagnostic data ([example_pub.py](./example_pub.py)), and one diagnostic aggregator configuration ([example.yaml](./example.yaml)) that provides analyzers aggregating it.
+This is a simple example to show the diagnostic_aggregator and add_analyzer in action. It involves one python script producing dummy diagnostic data ([example_pub.py](./example_pub.py)), one diagnostic aggregator configuration ([example_analyzers.yaml](./example_analyzers.yaml)) and one add_analyzer configuration ([example_add_analyzers.yaml](./example_add_analyzers.yaml)).
+
+The aggregator will launch and load all the analyzers listed in ([example_analyzers.yaml](./example_analyzers.yaml)). Then the aggregator will be notified that there are additional analyzers that we also want to load in ([example_add_analyzers.yaml](./example_add_analyzers.yaml)). After this reload all analyzers will be active.
 
 Run the example with `ros2 launch diagnostic_aggregator example.launch.py`

diagnostic_aggregator/example/example.launch.py.in

@@ -4,6 +4,7 @@ import launch
 import launch_ros.actions
 
 analyzer_params_filepath = "@ANALYZER_PARAMS_FILEPATH@"
+add_analyzer_params_filepath = "@ADD_ANALYZER_PARAMS_FILEPATH@"
 
 
 def generate_launch_description():
@@ -12,11 +13,18 @@ def generate_launch_description():
         executable='aggregator_node',
         output='screen',
         parameters=[analyzer_params_filepath])
+    add_analyzer = launch_ros.actions.Node(
+        package='diagnostic_aggregator',
+        executable='add_analyzer',
+        output='screen',
+        parameters=[add_analyzer_params_filepath]
+    )
     diag_publisher = launch_ros.actions.Node(
         package='diagnostic_aggregator',
         executable='example_pub.py')
     return launch.LaunchDescription([
         aggregator,
+        add_analyzer,
         diag_publisher,
         launch.actions.RegisterEventHandler(
             event_handler=launch.event_handlers.OnProcessExit(

diagnostic_aggregator/example/example_add_analyzers.yaml

@@ -0,0 +1,6 @@
+/**:
+  ros__parameters:
+    optional:
+      type: diagnostic_aggregator/GenericAnalyzer
+      path: Optional
+      contains: [ '/optional' ]

diagnostic_aggregator/example/example_pub.py

@@ -81,6 +81,10 @@ def __init__(self):
                              name='/sensors/front/cam', message='OK'),
             DiagnosticStatus(level=DiagnosticStatus.OK,
                              name='/sensors/rear/cam', message='OK'),
+
+            # Optional
+            DiagnosticStatus(level=DiagnosticStatus.OK,
+                             name='/optional/runtime/analyzer', message='OK'),
         ]
 
     def timer_callback(self):

diagnostic_aggregator/include/diagnostic_aggregator/aggregator.hpp

@@ -133,6 +133,8 @@ class Aggregator : public rclcpp::Node
   rclcpp::Service<diagnostic_msgs::srv::AddDiagnostics>::SharedPtr add_srv_;
   /// DiagnosticArray, /diagnostics
   rclcpp::Subscription<diagnostic_msgs::msg::DiagnosticArray>::SharedPtr diag_sub_;
+  /// ParameterEvent, /parameter_events
+  rclcpp::Subscription<rcl_interfaces::msg::ParameterEvent>::SharedPtr param_sub_;
   /// DiagnosticArray, /diagnostics_agg
   rclcpp::Publisher<diagnostic_msgs::msg::DiagnosticArray>::SharedPtr agg_pub_;
   /// DiagnosticStatus, /diagnostics_toplevel_state
@@ -165,6 +167,16 @@ class Aggregator : public rclcpp::Node
   /// Records all ROS warnings. No warnings are repeated.
   std::set<std::string> ros_warnings_;
 
+  /*
+   *!\brief Checks for new parameters to trigger reinitialization of the AnalyzerGroup and OtherAnalyzer
+   */
+  void parameterCallback(const rcl_interfaces::msg::ParameterEvent::SharedPtr param_msg);
+
+  /*
+   *!\brief (re)initializes the AnalyzerGroup and OtherAnalyzer
+   */
+  void initAnalyzers();
+
   /*
    *!\brief Checks timestamp of message, and warns if timestamp is 0 (not set)
    */

diagnostic_aggregator/include/diagnostic_aggregator/generic_analyzer_base.hpp

@@ -178,7 +178,7 @@ class GenericAnalyzerBase : public Analyzer
 
     auto header_status = std::make_shared<diagnostic_msgs::msg::DiagnosticStatus>();
     header_status->name = path_;
-    header_status->level = 0;
+    header_status->level = diagnostic_msgs::msg::DiagnosticStatus::OK;
     header_status->message = "OK";
 
     std::vector<std::shared_ptr<diagnostic_msgs::msg::DiagnosticStatus>> processed;
@@ -224,22 +224,28 @@ class GenericAnalyzerBase : public Analyzer
 
     // Header is not stale unless all subs are
     if (all_stale) {
-      header_status->level = diagnostic_msgs::msg::DiagnosticStatus::STALE;
+      // If we elect to discard stale items, then it signals that the absence of an item
+      // is not considered problematic, so we should allow empty queues to roll up as OK.
+      if (discard_stale_) {
+        header_status->level = diagnostic_msgs::msg::DiagnosticStatus::OK;
+      } else {
+        header_status->level = diagnostic_msgs::msg::DiagnosticStatus::STALE;
+      }
     } else if (header_status->level == diagnostic_msgs::msg::DiagnosticStatus::STALE) {
-      header_status->level = 2;
+      header_status->level = diagnostic_msgs::msg::DiagnosticStatus::ERROR;
     }
 
     header_status->message = valToMsg(header_status->level);
 
     // If we expect a given number of items, check that we have this number
     if (num_items_expected_ == 0 && items_.empty()) {
-      header_status->level = 0;
+      header_status->level = diagnostic_msgs::msg::DiagnosticStatus::OK;
       header_status->message = "OK";
     } else if (  // NOLINT
       num_items_expected_ > 0 &&
       static_cast<int8_t>(items_.size()) != num_items_expected_)
     {  // NOLINT
-      int8_t lvl = 2;
+      int8_t lvl = diagnostic_msgs::msg::DiagnosticStatus::ERROR;
       header_status->level = std::max(lvl, static_cast<int8_t>(header_status->level));
 
       std::stringstream expec, item;

diagnostic_aggregator/include/diagnostic_aggregator/other_analyzer.hpp

@@ -145,7 +145,7 @@ class OtherAnalyzer : public GenericAnalyzerBase
         processed.begin();
       for (; it != processed.end(); ++it) {
         if ((*it)->name == path_) {
-          (*it)->level = 2;
+          (*it)->level = diagnostic_msgs::msg::DiagnosticStatus::ERROR;
           (*it)->message = "Unanalyzed items found in \"Other\"";
           break;
         }