From ac97a0cba5efa4ab90e4cced576a7fdcb7e8992a Mon Sep 17 00:00:00 2001 From: Christian Henkel <6976069+ct2034@users.noreply.github.com> Date: Thu, 18 Jan 2024 23:03:31 +0100 Subject: [PATCH 01/13] formatting fixes from PR324 (#327) --- diagnostic_aggregator/README.md | 7 +++++-- diagnostic_aggregator/mainpage.dox | 9 +++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/diagnostic_aggregator/README.md b/diagnostic_aggregator/README.md index e7f63c2b7..08178a066 100644 --- a/diagnostic_aggregator/README.md +++ b/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 @@ -186,4 +189,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 \ No newline at end of file +TODO: Port tutorials #contributions-welcome diff --git a/diagnostic_aggregator/mainpage.dox b/diagnostic_aggregator/mainpage.dox index f7827de57..b720b86e4 100644 --- a/diagnostic_aggregator/mainpage.dox +++ b/diagnostic_aggregator/mainpage.dox @@ -16,7 +16,7 @@ See Analyzer for more information on the base class. \subsubsection generic_analyzer GenericAnalyzer -\b generic_analyzer holds the GenericAnalyzer class, which is the most basic of the Analyzer's. It is used by the diagnostic_aggregator/Aggregator to store, process and republish diagnostics data. The GenericAnalyzer is loaded by the pluginlib as a Analyzer plugin. It is the most basic of all Analyzer's. +\b generic_analyzer holds the GenericAnalyzer class, which is the most basic of the Analyzer's. It is used by the diagnostic_aggregator/Aggregator to store, process and republish diagnostics data. The GenericAnalyzer is loaded by the pluginlib as a Analyzer plugin. It is the most basic of all Analyzer's. \subsubsection analyzer_group AnalyzerGroup @@ -37,10 +37,10 @@ aggregator_node subscribes to "/diagnostics" and publishes an aggregated set of \subsubsection topics ROS topics Subscribes to: -- \b "/diagnostics": [diagnostics_msgs/DiagnosticArray] +- \b "/diagnostics": [diagnostics_msgs/DiagnosticArray] Publishes to: -- \b "/diagnostics_agg": [diagnostics_msgs/DiagnosticArray] +- \b "/diagnostics_agg": [diagnostics_msgs/DiagnosticArray] \subsubsection parameters ROS parameters @@ -49,6 +49,7 @@ Reads the following parameters from the parameter server - \b "~pub_rate" : \b double [optional] Rate that output diagnostics published - \b "~base_path" : \b double [optional] Prepended to all analyzed output - \b "~analyzers" : \b {} Configuration for loading analyzers +- \b "~critical" : \b bool [optional] React immediately to a degradation in diagnostic state \subsection analyzer_loader analyzer_loader @@ -61,4 +62,4 @@ Reads the following parameters from the parameter server - \b "~analyzers" : \b {} Configuration for loading and testing analyzers -*/ \ No newline at end of file +*/ From 183b7de119dcc9632410404950cfc64beb7de297 Mon Sep 17 00:00:00 2001 From: Christian Henkel <6976069+ct2034@users.noreply.github.com> Date: Wed, 24 Jan 2024 15:39:29 +0100 Subject: [PATCH 02/13] typo --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 521414bf7..727ac3af2 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ # Overview -The diagnostics system collects information about hardware drivers and robot hardware to make them availaible to users and operators. +The diagnostics system collects information about hardware drivers and robot hardware to make them available to users and operators. The diagnostics system contains tools to collect and analyze this data. The diagnostics system is build around the `/diagnostics` topic. The topic is used for `diagnostic_msgs/DiagnosticArray` messages. From a09c0b1f6f03880b520fcc0f8a09acc363711fcd Mon Sep 17 00:00:00 2001 From: Christian Henkel <6976069+ct2034@users.noreply.github.com> Date: Thu, 25 Jan 2024 15:38:09 +0100 Subject: [PATCH 03/13] including depdency (#322) --- diagnostic_updater/package.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/diagnostic_updater/package.xml b/diagnostic_updater/package.xml index 7cf5923a4..c84dca98a 100644 --- a/diagnostic_updater/package.xml +++ b/diagnostic_updater/package.xml @@ -32,6 +32,7 @@ ament_lint_common launch launch_testing + launch_testing_ros python3-pytest rclcpp_lifecycle From 1401f487c272d72a96a194a8371ce282a6fb599a Mon Sep 17 00:00:00 2001 From: Andrew Symington Date: Thu, 25 Jan 2024 06:39:14 -0800 Subject: [PATCH 04/13] Avoid rolling up an ERROR state when empty GenericAnalyzer blocks are marked discard_stale, or when all of their items are STALE. (#315) * If discard_stale = true, don't interpret stale as ERROR when rolling up * Check in test to exercise behavior --------- Signed-off-by: Andrew Symington Signed-off-by: Christian Henkel Co-authored-by: Christian Henkel --- diagnostic_aggregator/CMakeLists.txt | 5 + .../generic_analyzer_base.hpp | 16 +- .../diagnostic_aggregator/other_analyzer.hpp | 2 +- diagnostic_aggregator/package.xml | 1 + .../test/test_discard_behavior.py | 299 ++++++++++++++++++ 5 files changed, 317 insertions(+), 6 deletions(-) create mode 100644 diagnostic_aggregator/test/test_discard_behavior.py diff --git a/diagnostic_aggregator/CMakeLists.txt b/diagnostic_aggregator/CMakeLists.txt index fd6c2a070..12aac3b1f 100644 --- a/diagnostic_aggregator/CMakeLists.txt +++ b/diagnostic_aggregator/CMakeLists.txt @@ -128,6 +128,11 @@ if(BUILD_TESTING) 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( diff --git a/diagnostic_aggregator/include/diagnostic_aggregator/generic_analyzer_base.hpp b/diagnostic_aggregator/include/diagnostic_aggregator/generic_analyzer_base.hpp index 11a5b4f43..d7cb8a485 100644 --- a/diagnostic_aggregator/include/diagnostic_aggregator/generic_analyzer_base.hpp +++ b/diagnostic_aggregator/include/diagnostic_aggregator/generic_analyzer_base.hpp @@ -178,7 +178,7 @@ class GenericAnalyzerBase : public Analyzer auto header_status = std::make_shared(); header_status->name = path_; - header_status->level = 0; + header_status->level = diagnostic_msgs::msg::DiagnosticStatus::OK; header_status->message = "OK"; std::vector> 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(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(header_status->level)); std::stringstream expec, item; diff --git a/diagnostic_aggregator/include/diagnostic_aggregator/other_analyzer.hpp b/diagnostic_aggregator/include/diagnostic_aggregator/other_analyzer.hpp index 3cb9162e4..67d0f1b0a 100644 --- a/diagnostic_aggregator/include/diagnostic_aggregator/other_analyzer.hpp +++ b/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; } diff --git a/diagnostic_aggregator/package.xml b/diagnostic_aggregator/package.xml index 31095edd0..4e0b89a35 100644 --- a/diagnostic_aggregator/package.xml +++ b/diagnostic_aggregator/package.xml @@ -31,6 +31,7 @@ ament_cmake_pytest ament_lint_auto ament_lint_common + launch_pytest launch_testing_ament_cmake launch_testing_ros diff --git a/diagnostic_aggregator/test/test_discard_behavior.py b/diagnostic_aggregator/test/test_discard_behavior.py new file mode 100644 index 000000000..16dee0162 --- /dev/null +++ b/diagnostic_aggregator/test/test_discard_behavior.py @@ -0,0 +1,299 @@ +# Copyright 2023 Open Source Robotics Foundation, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# DESCRIPTION +# This test ensures that a parent AnalyzerGroup does not roll up an ERROR state when a +# GenericAnalyzer child block is marked with discard_stale: true and either of these +# conditions are met: +# +# 1. There are no statuses that match any of the GenericAnalyzer child blocks. +# 2. Every matching status in the GenericAnalyzer child block has been marked stale +# +# In this example, if foo and bar have no matching statuses or all of their statuses +# are STALE, they will roll up as OK because the discard_stale: true flag implies that +# stale statuses are disposable. +# +# analyzer: +# ros__parameters: +# path: 'agg' +# pub_rate: 1.0 +# analyzers: +# part: +# type: 'diagnostic_aggregator/AnalyzerGroup' +# path: 'part' +# foo: +# type: 'diagnostic_aggregator/GenericAnalyzer' +# path: 'foo' +# find_and_remove_prefix: ['/foo:'] +# num_items: 1 +# bar: +# type: 'diagnostic_aggregator/GenericAnalyzer' +# path: 'bar' +# find_and_remove_prefix: ['/bar:'] +# discard_stale: true + +# Python includes. +from collections import namedtuple +import tempfile + +# ROS2 includes. +from diagnostic_msgs.msg import DiagnosticArray, DiagnosticStatus +import launch +import launch_pytest +import launch_ros +import pytest +import rclpy +from rclpy.executors import SingleThreadedExecutor +from rclpy.node import Node +from rclpy.task import Future + + +# All tests take a common structure. +TestMetadata = namedtuple( + 'TestMetadata', + ['foo_discard', 'foo_status', 'bar_discard', 'bar_status', 'agg_expected'], +) + +# A status value of 'None' means that the state is never sent (it's missing). +TEST_METADATA = [ + # CASE 1: both 'foo' and 'bar' are marked discard_stale := true + TestMetadata( + foo_discard=True, + foo_status=None, + bar_discard=True, + bar_status=None, + agg_expected=DiagnosticStatus.OK, + ), + TestMetadata( + foo_discard=True, + foo_status=DiagnosticStatus.STALE, + bar_discard=True, + bar_status=DiagnosticStatus.STALE, + agg_expected=DiagnosticStatus.OK, + ), + TestMetadata( + foo_discard=True, + foo_status=None, + bar_discard=True, + bar_status=DiagnosticStatus.STALE, + agg_expected=DiagnosticStatus.OK, + ), + # CASE 2: both 'foo' and 'bar' are marked discard_stale := false + TestMetadata( + foo_discard=False, + foo_status=None, + bar_discard=False, + bar_status=None, + agg_expected=DiagnosticStatus.STALE, + ), + TestMetadata( + foo_discard=False, + foo_status=DiagnosticStatus.STALE, + bar_discard=False, + bar_status=DiagnosticStatus.STALE, + agg_expected=DiagnosticStatus.STALE, + ), + TestMetadata( + foo_discard=False, + foo_status=None, + bar_discard=False, + bar_status=DiagnosticStatus.STALE, + agg_expected=DiagnosticStatus.STALE, + ), + # CASE 3: one of 'foo' or 'bar' are marked discard_stale := true + TestMetadata( + foo_discard=True, + foo_status=None, + bar_discard=False, + bar_status=None, + agg_expected=DiagnosticStatus.ERROR, # <-- This is the case we are testing for. + # if one of the children is *not* marked discard_stale := true and + # there are no statuses, then the parent should roll up to ERROR. + ), + TestMetadata( + foo_discard=True, + foo_status=DiagnosticStatus.OK, + bar_discard=False, + bar_status=None, + agg_expected=DiagnosticStatus.ERROR, + ), + TestMetadata( + foo_discard=True, + foo_status=None, + bar_discard=False, + bar_status=DiagnosticStatus.OK, + agg_expected=DiagnosticStatus.OK, # <-- This is the case we are testing for. + # but if a child is marked discard_stale := true and there are no statuses, + # the parent should roll up to OK. + ), + TestMetadata( + foo_discard=True, + foo_status=DiagnosticStatus.OK, + bar_discard=False, + bar_status=DiagnosticStatus.OK, + agg_expected=DiagnosticStatus.OK, + ), +] + + +class DiagnosticsTestNode(Node): + """Class that publishes raw diagnostics and listens for aggregated diagnostics.""" + + def __init__(self, foo_status, bar_status, agg_expected): + super().__init__(node_name='diagnostics_listener_node') + self.foo_status = foo_status + self.bar_status = bar_status + self.agg_expected = agg_expected + self.agg_received = None + self.counter = 0 + self.future = Future() + self.subscriber = self.create_subscription( + msg_type=DiagnosticArray, + topic='/diagnostics_agg', + callback=self.diagnostics_aggregated_callback, + qos_profile=10, + ) + self.publisher = self.create_publisher( + msg_type=DiagnosticArray, topic='/diagnostics', qos_profile=10 + ) + self.timer = self.create_timer(0.1, self.timer_callback) + + def timer_callback(self): + """Call from a timer to send off raw diagnostics.""" + msg = DiagnosticArray() + msg.header.stamp = self.get_clock().now().to_msg() + msg.header.frame_id = 'robot' + if self.foo_status is not None: + msg.status.append( + DiagnosticStatus( + name='/foo', level=self.foo_status, message='Foo', hardware_id='foo' + ) + ) + if self.bar_status is not None: + msg.status.append( + DiagnosticStatus( + name='/bar', level=self.bar_status, message='Bar', hardware_id='bar' + ) + ) + self.publisher.publish(msg) + + def diagnostics_aggregated_callback(self, msg): + """Call from a subscriber providing aggregated diagnostics.""" + for status in msg.status: + if status.name == '/robot/agg': + self.agg_received = status.level + self.counter += 1 + if self.agg_expected == status.level: + # Diagnostics may take a few iterations to 'settle' into the right + # state because of how the STALE logic is applied. So, we keep checking + # the aggregator result until it reaches the value we are expecting, + # and then trigger the future. + self.future.set_result(self.counter) + + +@pytest.fixture(scope='function') +def test_metadata(request): + """Enable parameter indirection, so we can pass a parameterization into fixtures.""" + return request.param + + +@pytest.fixture(scope='function') +def yaml_file(test_metadata): + """Generate a YAML file to test a specific configuration state.""" + with tempfile.NamedTemporaryFile(delete=False) as fp: + fp.write( + bytes( + f""" +diagnostic_aggregator: + ros__parameters: + path: 'robot' + pub_rate: 1.0 + analyzers: + part: + type: 'diagnostic_aggregator/AnalyzerGroup' + path: 'agg' + timeout: 2.0 + foo: + type: 'diagnostic_aggregator/GenericAnalyzer' + path: 'foo' + find_and_remove_prefix: ['/foo'] + discard_stale: {test_metadata.foo_discard} + bar: + type: 'diagnostic_aggregator/GenericAnalyzer' + path: 'bar' + find_and_remove_prefix: ['/bar'] + discard_stale: {test_metadata.bar_discard} +""", + 'utf-8', + ) + ) + return fp.name + + +@pytest.fixture(scope='function') +def diagnostic_aggregator_node(): + """Declare an aggregator that uses a global configuration set by the launch.""" + return launch_ros.actions.Node( + name='diagnostic_aggregator', + package='diagnostic_aggregator', + executable='aggregator_node', + ) + + +@launch_pytest.fixture(scope='function') +def launch_description(yaml_file, diagnostic_aggregator_node): + """Declare what should be launched in each test.""" + return launch.LaunchDescription( + [ + launch_ros.actions.SetParametersFromFile(yaml_file), + diagnostic_aggregator_node, + launch_pytest.actions.ReadyToTest(), + ] + ) + + +@pytest.mark.parametrize('test_metadata', TEST_METADATA, indirect=True) +@pytest.mark.launch(fixture=launch_description) +def test_discard_behavior(test_metadata, launch_context): + """Run a launch test for each test in our set of tests.""" + rclpy.init() + + node = DiagnosticsTestNode( + foo_status=test_metadata.foo_status, + bar_status=test_metadata.bar_status, + agg_expected=test_metadata.agg_expected, + ) + + executor = SingleThreadedExecutor() + executor.add_node(node) + + try: + executor.spin_until_future_complete(future=node.future, timeout_sec=10.0) + print( + f""" + The test produced the following result: + + foo_level: {test_metadata.foo_status} (discard: {test_metadata.foo_discard}) + + bar_level: {test_metadata.bar_status} (discard: {test_metadata.bar_discard}) + Expected level: {test_metadata.agg_expected} + Received level: {node.agg_received} + """ + ) + assert node.future.done(), 'Launch timed out without producing aggregation' + assert ( + node.agg_received == test_metadata.agg_expected + ), 'Unexpected parent status level' + print(f'It took {node.future.result()} aggregations to find the correct status') + + finally: + rclpy.try_shutdown() From b1c8dd16694869b9bc8d778ef0386b9f5dba7437 Mon Sep 17 00:00:00 2001 From: Christian Henkel Date: Thu, 21 Mar 2024 23:20:28 +0100 Subject: [PATCH 05/13] documentation on new branch Signed-off-by: Christian Henkel --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 727ac3af2..70032ebf6 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,10 @@ The [`ros2` branch](https://github.com/ros/diagnostics/tree/ros2) targets - *Iron Irwini* and - *Rolling Ridley* +The [`ros2-jazzy` branch](https://github.com/ros/diagnostics/tree/ros2-jazzy) targets + +- *Jazzy Jalisco* + # License The source code is released under a [BSD 3-Clause license](LICENSE). From f89beba1b2c8537399e0e38986d6f2356d5a1ebd Mon Sep 17 00:00:00 2001 From: Richard <47382675+RichardvdK@users.noreply.github.com> Date: Fri, 22 Mar 2024 11:37:13 +0100 Subject: [PATCH 06/13] Port cpu_monitor to ROS2 (#326) * add cpu monitor to the common diagnostics * add 3 unittests * add psutil to package xml * add debug print for cpu percentages and change checking percentage to -1 * update code with review comments. Added a SetUp() method in the test and move the sleep to there to make sure this is done every test in a systemactic way --------- Co-authored-by: Richardvdketterij --- diagnostic_common_diagnostics/CMakeLists.txt | 7 +- diagnostic_common_diagnostics/README.md | 32 ++++- .../cpu_monitor.py | 119 ++++++++++++++++++ diagnostic_common_diagnostics/mainpage.dox | 1 + diagnostic_common_diagnostics/package.xml | 6 +- .../test/systemtest/test_cpu_monitor.py | 116 +++++++++++++++++ 6 files changed, 271 insertions(+), 10 deletions(-) create mode 100755 diagnostic_common_diagnostics/diagnostic_common_diagnostics/cpu_monitor.py create mode 100644 diagnostic_common_diagnostics/test/systemtest/test_cpu_monitor.py diff --git a/diagnostic_common_diagnostics/CMakeLists.txt b/diagnostic_common_diagnostics/CMakeLists.txt index 65720a08e..5c0f11f6b 100644 --- a/diagnostic_common_diagnostics/CMakeLists.txt +++ b/diagnostic_common_diagnostics/CMakeLists.txt @@ -8,6 +8,7 @@ find_package(ament_cmake_python REQUIRED) ament_python_install_package(${PROJECT_NAME}) install(PROGRAMS + ${PROJECT_NAME}/cpu_monitor.py ${PROJECT_NAME}/ntp_monitor.py DESTINATION lib/${PROJECT_NAME} ) @@ -17,10 +18,14 @@ if(BUILD_TESTING) ament_lint_auto_find_test_dependencies() find_package(ament_cmake_pytest REQUIRED) + ament_add_pytest_test( + test_cpu_monitor + test/systemtest/test_cpu_monitor.py + TIMEOUT 10) ament_add_pytest_test( test_ntp_monitor test/systemtest/test_ntp_monitor.py TIMEOUT 10) endif() -ament_package() \ No newline at end of file +ament_package() diff --git a/diagnostic_common_diagnostics/README.md b/diagnostic_common_diagnostics/README.md index 452d163ed..42df1c8dc 100644 --- a/diagnostic_common_diagnostics/README.md +++ b/diagnostic_common_diagnostics/README.md @@ -7,8 +7,31 @@ Currently only the NTP monitor is ported to ROS2. # Nodes +## cpu_monitor.py +The `cpu_monitor` module allows users to monitor the CPU usage of their system in real-time. +It publishes the usage percentage in a diagnostic message. + +* Name of the node is "cpu_monitor_" + hostname. +* Uses the following args: + * warning_percentage: If the CPU usage is > warning_percentage, a WARN status will be publised. + * window: the maximum length of the used collections.deque for queuing CPU readings. + +### Published Topics +#### /diagnostics +diagnostic_msgs/DiagnosticArray +The diagnostics information. + +### Parameters +#### warning_percentage +(default: 90) +warning percentage threshold. + +#### window +(default: 1) +Length of CPU readings queue. + ## ntp_monitor.py -Runs 'ntpdate' to check if the system clock is synchronized with the NTP server. +Runs 'ntpdate' to check if the system clock is synchronized with the NTP server. * If the offset is smaller than `offset-tolerance`, an `OK` status will be published. * If the offset is larger than the configured `offset-tolerance`, a `WARN` status will be published, * if it is bigger than `error-offset-tolerance`, an `ERROR` status will be published. @@ -20,7 +43,7 @@ diagnostic_msgs/DiagnosticArray The diagnostics information. ### Parameters -#### ntp_hostname +#### ntp_hostname (default: "pool.ntp.org") Hostname of NTP server. @@ -46,9 +69,6 @@ Disable self test. ## hd_monitor.py **To be ported** -## cpu_monitor.py -**To be ported** - ## ram_monitor.py **To be ported** @@ -56,4 +76,4 @@ Disable self test. **To be ported** ## tf_monitor.py -**To be ported** \ No newline at end of file +**To be ported** diff --git a/diagnostic_common_diagnostics/diagnostic_common_diagnostics/cpu_monitor.py b/diagnostic_common_diagnostics/diagnostic_common_diagnostics/cpu_monitor.py new file mode 100755 index 000000000..866629572 --- /dev/null +++ b/diagnostic_common_diagnostics/diagnostic_common_diagnostics/cpu_monitor.py @@ -0,0 +1,119 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +# +# Software License Agreement (BSD License) +# +# Copyright (c) 2017, TNO IVS, Helmond, Netherlands +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following +# disclaimer in the documentation and/or other materials provided +# with the distribution. +# * Neither the name of the TNO IVS nor the names of its +# contributors may be used to endorse or promote products derived +# from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +# COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN +# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +# \author Rein Appeldoorn + +from collections import deque +import socket +import traceback + +from diagnostic_msgs.msg import DiagnosticStatus + +from diagnostic_updater import DiagnosticTask, Updater + +import psutil + +import rclpy +from rclpy.node import Node + + +class CpuTask(DiagnosticTask): + + def __init__(self, warning_percentage=90, window=1): + DiagnosticTask.__init__(self, 'CPU Information') + + self._warning_percentage = int(warning_percentage) + self._readings = deque(maxlen=window) + + def _get_average_reading(self): + def avg(lst): + return float(sum(lst)) / len(lst) if lst else float('nan') + + return [avg(cpu_percentages) + for cpu_percentages in zip(*self._readings)] + + def run(self, stat): + self._readings.append(psutil.cpu_percent(percpu=True)) + cpu_percentages = self._get_average_reading() + cpu_average = sum(cpu_percentages) / len(cpu_percentages) + + stat.add('CPU Load Average', f'{cpu_average:.2f}') + + warn = False + for idx, cpu_percentage in enumerate(cpu_percentages): + stat.add(f'CPU {idx} Load', f'{cpu_percentage:.2f}') + if cpu_percentage > self._warning_percentage: + warn = True + + if warn: + stat.summary(DiagnosticStatus.WARN, + f'At least one CPU exceeds {self._warning_percentage} percent') + else: + stat.summary(DiagnosticStatus.OK, + f'CPU Average {cpu_average:.2f} percent') + + return stat + + +def main(args=None): + rclpy.init(args=args) + + # Create the node + hostname = socket.gethostname() + node = Node(f'cpu_monitor_{hostname.replace("-", "_")}') + + # Declare and get parameters + node.declare_parameter('warning_percentage', 90) + node.declare_parameter('window', 1) + + warning_percentage = node.get_parameter( + 'warning_percentage').get_parameter_value().integer_value + window = node.get_parameter('window').get_parameter_value().integer_value + + # Create diagnostic updater with default updater rate of 1 hz + updater = Updater(node) + updater.setHardwareID(hostname) + updater.add(CpuTask(warning_percentage=warning_percentage, window=window)) + + rclpy.spin(node) + + +if __name__ == '__main__': + try: + main() + except KeyboardInterrupt: + pass + except Exception: + traceback.print_exc() diff --git a/diagnostic_common_diagnostics/mainpage.dox b/diagnostic_common_diagnostics/mainpage.dox index 25ee9c863..7aa872cef 100644 --- a/diagnostic_common_diagnostics/mainpage.dox +++ b/diagnostic_common_diagnostics/mainpage.dox @@ -4,6 +4,7 @@ \b diagnostic_common_diagnostics contains a few common diagnostic nodes +- cpu_monitor publishes diagnostic messages with the CPU usage of the system. - ntp_monitor publishes diagnostic messages for how well the NTP time sync is working. - tf_monitor used to publish diagnostic messages reporting on the health of the TF tree. It is based on tfwtf. It is not ported to ROS2. diff --git a/diagnostic_common_diagnostics/package.xml b/diagnostic_common_diagnostics/package.xml index 9d50bbe33..f6bad9205 100644 --- a/diagnostic_common_diagnostics/package.xml +++ b/diagnostic_common_diagnostics/package.xml @@ -18,20 +18,20 @@ ament_cmake ament_cmake_python - rclpy diagnostic_updater python3-ntplib + python3-psutil ament_lint_auto ament_cmake_xmllint ament_cmake_lint_cmake - + ament_cmake_pytest diff --git a/diagnostic_common_diagnostics/test/systemtest/test_cpu_monitor.py b/diagnostic_common_diagnostics/test/systemtest/test_cpu_monitor.py new file mode 100644 index 000000000..28430c482 --- /dev/null +++ b/diagnostic_common_diagnostics/test/systemtest/test_cpu_monitor.py @@ -0,0 +1,116 @@ +# -*- coding: utf-8 -*- +# Software License Agreement (BSD License) +# +# Copyright (c) 2023, Robert Bosch GmbH +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following +# disclaimer in the documentation and/or other materials provided +# with the distribution. +# * Neither the name of the Willow Garage nor the names of its +# contributors may be used to endorse or promote products derived +# from this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +# COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN +# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. + +import time +import unittest + +from diagnostic_common_diagnostics.cpu_monitor import CpuTask + +from diagnostic_msgs.msg import DiagnosticStatus + +from diagnostic_updater import DiagnosticArray, Updater +from diagnostic_updater import DiagnosticStatusWrapper + +import rclpy +from rclpy.node import Node + + +class TestCPUMonitor(unittest.TestCase): + + @classmethod + def setUpClass(cls): + rclpy.init(args=None) + + @classmethod + def tearDownClass(cls): + if rclpy.ok(): + rclpy.shutdown() + + def setUp(self): + # In this case is recommended for accuracy that psutil.cpu_percent() + # function be called with at least 0.1 seconds between calls. + time.sleep(0.1) + + def diagnostics_callback(self, msg): + self.message_recieved = True + self.assertEqual(len(msg.status), 1) + + def test_ok(self): + warning_percentage = 100 + task = CpuTask(warning_percentage) + stat = DiagnosticStatusWrapper() + task.run(stat) + self.assertEqual(task.name, 'CPU Information') + self.assertEqual(stat.level, DiagnosticStatus.OK) + self.assertIn(str('CPU Average'), stat.message) + + # Check for at least 1 CPU Load Average and 1 CPU Load + self.assertGreaterEqual(len(stat.values), 2) + + def test_warn(self): + warning_percentage = -1 + task = CpuTask(warning_percentage) + stat = DiagnosticStatusWrapper() + task.run(stat) + print(f'Raw readings: {task._readings}') + self.assertEqual(task.name, 'CPU Information') + self.assertEqual(stat.level, DiagnosticStatus.WARN) + self.assertIn(str('At least one CPU exceeds'), stat.message) + + # Check for at least 1 CPU Load Average and 1 CPU Load + self.assertGreaterEqual(len(stat.values), 2) + + def test_updater(self): + self.message_recieved = False + + node = Node('cpu_monitor_test') + updater = Updater(node) + updater.setHardwareID('test_id') + updater.add(CpuTask()) + + node.create_subscription( + DiagnosticArray, '/diagnostics', self.diagnostics_callback, 10) + + start_time = time.time() + timeout = 5.0 # Timeout in seconds + + while not self.message_recieved: + rclpy.spin_once(node) + time.sleep(0.1) + elapsed_time = time.time() - start_time + if elapsed_time >= timeout: + self.fail('No diagnostics received') + + +if __name__ == '__main__': + unittest.main() From 83c030e02514c185c91cea4b63da662566cb7039 Mon Sep 17 00:00:00 2001 From: Christian Henkel Date: Fri, 22 Mar 2024 13:56:40 +0100 Subject: [PATCH 07/13] Rolling obviously also uses the jazzy branch Signed-off-by: Christian Henkel --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 70032ebf6..96c20ae59 100644 --- a/README.md +++ b/README.md @@ -37,12 +37,12 @@ Diagnostics messages that are not aggregated can be visualized by [`rqt_runtime_ The [`ros2` branch](https://github.com/ros/diagnostics/tree/ros2) targets - *Humble Hawksbill* -- *Iron Irwini* and -- *Rolling Ridley* +- *Iron Irwini* The [`ros2-jazzy` branch](https://github.com/ros/diagnostics/tree/ros2-jazzy) targets - *Jazzy Jalisco* +- *Rolling Ridley* # License From 07ed478a65a6b94bdf5419d41f6ee9c5a102a8d1 Mon Sep 17 00:00:00 2001 From: Christian Henkel Date: Fri, 22 Mar 2024 14:16:26 +0100 Subject: [PATCH 08/13] changelogs Signed-off-by: Christian Henkel --- diagnostic_aggregator/CHANGELOG.rst | 10 ++++++++++ diagnostic_common_diagnostics/CHANGELOG.rst | 8 ++++++++ diagnostic_updater/CHANGELOG.rst | 9 +++++++++ diagnostics/CHANGELOG.rst | 3 +++ self_test/CHANGELOG.rst | 5 +++++ 5 files changed, 35 insertions(+) diff --git a/diagnostic_aggregator/CHANGELOG.rst b/diagnostic_aggregator/CHANGELOG.rst index 9c48d2f13..8cf482f68 100644 --- a/diagnostic_aggregator/CHANGELOG.rst +++ b/diagnostic_aggregator/CHANGELOG.rst @@ -2,6 +2,16 @@ Changelog for package diagnostic_aggregator ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- +* Avoid rolling up an ERROR state when empty GenericAnalyzer blocks are marked discard_stale, or when all of their items are STALE. (`#315 `_) +* formatting fixes from PR324 (`#327 `_) +* Debugging instability introduced by `#317 `_ (`#323 `_) +* feat: publish top level msg when error is received (`#317 `_) +* Empty default aggregator base_path (`#305 `_) +* using defined state for stale (`#298 `_) +* Contributors: Andrew Symington, Christian Henkel, outrider-jhulas + 3.1.2 (2023-03-24) ------------------ diff --git a/diagnostic_common_diagnostics/CHANGELOG.rst b/diagnostic_common_diagnostics/CHANGELOG.rst index 506989c68..958b01459 100644 --- a/diagnostic_common_diagnostics/CHANGELOG.rst +++ b/diagnostic_common_diagnostics/CHANGELOG.rst @@ -2,6 +2,14 @@ Changelog for package diagnostic_common_diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- +* Port cpu_monitor to ROS2 (`#326 `_) +* Debugging instability introduced by `#317 `_ (`#323 `_) +* not testing on foxy any more (`#310 `_) +* Iron support (`#304 `_) +* Contributors: Christian Henkel, Richard + 3.1.2 (2023-03-24) ------------------ * replacing ntpdate with ntplib (`#289 `_) diff --git a/diagnostic_updater/CHANGELOG.rst b/diagnostic_updater/CHANGELOG.rst index a4e692168..f85ecba49 100644 --- a/diagnostic_updater/CHANGELOG.rst +++ b/diagnostic_updater/CHANGELOG.rst @@ -2,6 +2,15 @@ Changelog for package diagnostic_updater ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- +* including depdency (`#322 `_) +* Debugging instability introduced by `#317 `_ (`#323 `_) +* feat: add param to use fqn in updater (`#320 `_) +* fix: method names & verbose logging (`#307 `_) +* Fix diagnostic_updater timestamps (`#299 `_) +* Contributors: Christian Henkel, Kevin Schwarzer, h-wata, outrider-jhulas + 3.1.2 (2023-03-24) ------------------ diff --git a/diagnostics/CHANGELOG.rst b/diagnostics/CHANGELOG.rst index 9e45ec832..992fbc2f3 100644 --- a/diagnostics/CHANGELOG.rst +++ b/diagnostics/CHANGELOG.rst @@ -2,6 +2,9 @@ Changelog for package diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- + 3.1.2 (2023-03-24) ------------------ diff --git a/self_test/CHANGELOG.rst b/self_test/CHANGELOG.rst index fe0c3e22f..ed2779987 100644 --- a/self_test/CHANGELOG.rst +++ b/self_test/CHANGELOG.rst @@ -2,6 +2,11 @@ Changelog for package self_test ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- +* Self test publishes the service under the node name, again (`#269 `_) +* Contributors: Christian Henkel + 3.1.2 (2023-03-24) ------------------ From c1b908cd83554a930d9076560041a8b1173252e9 Mon Sep 17 00:00:00 2001 From: Christian Henkel Date: Fri, 22 Mar 2024 15:18:21 +0100 Subject: [PATCH 09/13] 3.2.0 --- diagnostic_aggregator/CHANGELOG.rst | 4 ++-- diagnostic_aggregator/package.xml | 2 +- diagnostic_common_diagnostics/CHANGELOG.rst | 4 ++-- diagnostic_common_diagnostics/package.xml | 2 +- diagnostic_updater/CHANGELOG.rst | 4 ++-- diagnostic_updater/package.xml | 2 +- diagnostics/CHANGELOG.rst | 4 ++-- diagnostics/package.xml | 2 +- self_test/CHANGELOG.rst | 4 ++-- self_test/package.xml | 2 +- 10 files changed, 15 insertions(+), 15 deletions(-) diff --git a/diagnostic_aggregator/CHANGELOG.rst b/diagnostic_aggregator/CHANGELOG.rst index 8cf482f68..efda16d09 100644 --- a/diagnostic_aggregator/CHANGELOG.rst +++ b/diagnostic_aggregator/CHANGELOG.rst @@ -2,8 +2,8 @@ Changelog for package diagnostic_aggregator ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Forthcoming ------------ +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 `_) * formatting fixes from PR324 (`#327 `_) * Debugging instability introduced by `#317 `_ (`#323 `_) diff --git a/diagnostic_aggregator/package.xml b/diagnostic_aggregator/package.xml index 4e0b89a35..677e04365 100644 --- a/diagnostic_aggregator/package.xml +++ b/diagnostic_aggregator/package.xml @@ -2,7 +2,7 @@ diagnostic_aggregator - 3.1.2 + 3.2.0 diagnostic_aggregator Austin Hendrix Brice Rebsamen diff --git a/diagnostic_common_diagnostics/CHANGELOG.rst b/diagnostic_common_diagnostics/CHANGELOG.rst index 958b01459..0c53e9ca1 100644 --- a/diagnostic_common_diagnostics/CHANGELOG.rst +++ b/diagnostic_common_diagnostics/CHANGELOG.rst @@ -2,8 +2,8 @@ Changelog for package diagnostic_common_diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Forthcoming ------------ +3.2.0 (2024-03-22) +------------------ * Port cpu_monitor to ROS2 (`#326 `_) * Debugging instability introduced by `#317 `_ (`#323 `_) * not testing on foxy any more (`#310 `_) diff --git a/diagnostic_common_diagnostics/package.xml b/diagnostic_common_diagnostics/package.xml index f6bad9205..de8808d70 100644 --- a/diagnostic_common_diagnostics/package.xml +++ b/diagnostic_common_diagnostics/package.xml @@ -2,7 +2,7 @@ diagnostic_common_diagnostics - 3.1.2 + 3.2.0 diagnostic_common_diagnostics Austin Hendrix Brice Rebsamen diff --git a/diagnostic_updater/CHANGELOG.rst b/diagnostic_updater/CHANGELOG.rst index f85ecba49..e5d4da069 100644 --- a/diagnostic_updater/CHANGELOG.rst +++ b/diagnostic_updater/CHANGELOG.rst @@ -2,8 +2,8 @@ Changelog for package diagnostic_updater ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Forthcoming ------------ +3.2.0 (2024-03-22) +------------------ * including depdency (`#322 `_) * Debugging instability introduced by `#317 `_ (`#323 `_) * feat: add param to use fqn in updater (`#320 `_) diff --git a/diagnostic_updater/package.xml b/diagnostic_updater/package.xml index c84dca98a..6935d9acb 100644 --- a/diagnostic_updater/package.xml +++ b/diagnostic_updater/package.xml @@ -2,7 +2,7 @@ diagnostic_updater - 3.1.2 + 3.2.0 diagnostic_updater contains tools for easily updating diagnostics. it is commonly used in device drivers to keep track of the status of output topics, device status, etc. Austin Hendrix Brice Rebsamen diff --git a/diagnostics/CHANGELOG.rst b/diagnostics/CHANGELOG.rst index 992fbc2f3..1f73b1330 100644 --- a/diagnostics/CHANGELOG.rst +++ b/diagnostics/CHANGELOG.rst @@ -2,8 +2,8 @@ Changelog for package diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Forthcoming ------------ +3.2.0 (2024-03-22) +------------------ 3.1.2 (2023-03-24) ------------------ diff --git a/diagnostics/package.xml b/diagnostics/package.xml index c581103e0..9445c1f93 100644 --- a/diagnostics/package.xml +++ b/diagnostics/package.xml @@ -2,7 +2,7 @@ diagnostics - 3.1.2 + 3.2.0 diagnostics Austin Hendrix Brice Rebsamen diff --git a/self_test/CHANGELOG.rst b/self_test/CHANGELOG.rst index ed2779987..1ebff1e4a 100644 --- a/self_test/CHANGELOG.rst +++ b/self_test/CHANGELOG.rst @@ -2,8 +2,8 @@ Changelog for package self_test ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -Forthcoming ------------ +3.2.0 (2024-03-22) +------------------ * Self test publishes the service under the node name, again (`#269 `_) * Contributors: Christian Henkel diff --git a/self_test/package.xml b/self_test/package.xml index 600cc9cd4..6667d9b8f 100644 --- a/self_test/package.xml +++ b/self_test/package.xml @@ -2,7 +2,7 @@ self_test - 3.1.2 + 3.2.0 self_test Austin Hendrix Brice Rebsamen From 50770166eb7f5cead0687239d5c84f4712c2683f Mon Sep 17 00:00:00 2001 From: Christian Henkel Date: Fri, 22 Mar 2024 16:40:45 +0100 Subject: [PATCH 10/13] trying fix Signed-off-by: Christian Henkel --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 2fe97d371..407b1ddbc 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -31,7 +31,7 @@ jobs: os: ubuntu-22.04 runs-on: ${{ matrix.os }} steps: - - uses: ros-tooling/setup-ros@master + - uses: ct2034/setup-ros@patch-1 - run: | sudo pip install pydocstyle==6.1.1 # downgrade to fix https://github.com/ament/ament_lint/pull/428 sudo pip install pip --upgrade From 194753aa87ef2998bd1d65927e5a9ebf46534c21 Mon Sep 17 00:00:00 2001 From: Christian Henkel Date: Fri, 22 Mar 2024 16:42:59 +0100 Subject: [PATCH 11/13] Revert "trying fix" This reverts commit 50770166eb7f5cead0687239d5c84f4712c2683f. --- .github/workflows/test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 407b1ddbc..2fe97d371 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -31,7 +31,7 @@ jobs: os: ubuntu-22.04 runs-on: ${{ matrix.os }} steps: - - uses: ct2034/setup-ros@patch-1 + - 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 From 8ee564091a06c980c50188ed6a8404e3a5484d71 Mon Sep 17 00:00:00 2001 From: Christian Henkel <6976069+ct2034@users.noreply.github.com> Date: Thu, 28 Mar 2024 09:37:05 +0100 Subject: [PATCH 12/13] Building in docker (#335) * runs on container * all on ubuntu latest * pydocstyle fix is obsolete * no uncrustify for noble --------- Signed-off-by: Christian Henkel --- .github/workflows/lint.yaml | 2 +- .github/workflows/test.yaml | 17 +++++++++-------- self_test/CMakeLists.txt | 4 ++++ 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/.github/workflows/lint.yaml b/.github/workflows/lint.yaml index 010a90157..3488e94ea 100644 --- a/.github/workflows/lint.yaml +++ b/.github/workflows/lint.yaml @@ -22,7 +22,7 @@ jobs: uncrustify, xmllint, ] - runs-on: ubuntu-22.04 + runs-on: ubuntu-latest env: AMENT_CPPCHECK_ALLOW_SLOW_VERSIONS: 1 steps: diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 2fe97d371..4d1497641 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -24,18 +24,19 @@ jobs: distro: [humble, iron, rolling] include: - distro: humble - os: ubuntu-22.04 + os: 22.04 - distro: iron - os: ubuntu-22.04 + os: 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' + # - run: | + # sudo apt install -y python3-pip + # pip3 install --break-system-packages 'flake8<5' # fix flake8.exceptions.FailedToLoadPlugin: Flake8 failed to load plugin "pycodestyle" due to cannot import name 'missing_whitespace_around_operator' from 'pycodestyle' (/usr/lib/python3/dist-packages/pycodestyle.py). + # if: ${{ matrix.os == '24.04' }} - uses: ros-tooling/action-ros-ci@master with: target-ros2-distro: ${{ matrix.distro }} diff --git a/self_test/CMakeLists.txt b/self_test/CMakeLists.txt index 0acbaa640..46401f9ed 100644 --- a/self_test/CMakeLists.txt +++ b/self_test/CMakeLists.txt @@ -67,6 +67,10 @@ if(BUILD_TESTING) set(ament_cmake_copyright_FOUND TRUE) ament_lint_auto_find_test_dependencies() + list(APPEND AMENT_LINT_AUTO_EXCLUDE + ament_cmake_uncrustify # Inconsistent between jammy and noble + ) + add_subdirectory(test) endif() From 431926f647cfc54ffbf729cd9c098b1764604d66 Mon Sep 17 00:00:00 2001 From: Christian Henkel <6976069+ct2034@users.noreply.github.com> Date: Thu, 28 Mar 2024 10:17:56 +0100 Subject: [PATCH 13/13] Fixing ntp launchtest (#330) - using a launchtest --------- Signed-off-by: Christian Henkel --- .github/workflows/test.yaml | 4 - diagnostic_common_diagnostics/CMakeLists.txt | 9 +- .../ntp_monitor.py | 20 ++- diagnostic_common_diagnostics/package.xml | 1 + .../test/systemtest/test_ntp_monitor.py | 130 ------------------ .../systemtest/test_ntp_monitor_launchtest.py | 108 +++++++++------ 6 files changed, 88 insertions(+), 184 deletions(-) delete mode 100644 diagnostic_common_diagnostics/test/systemtest/test_ntp_monitor.py diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 4d1497641..1e07c0adc 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -33,10 +33,6 @@ jobs: container: ubuntu:${{ matrix.os }} steps: - uses: ros-tooling/setup-ros@master - # - run: | - # sudo apt install -y python3-pip - # pip3 install --break-system-packages 'flake8<5' # fix flake8.exceptions.FailedToLoadPlugin: Flake8 failed to load plugin "pycodestyle" due to cannot import name 'missing_whitespace_around_operator' from 'pycodestyle' (/usr/lib/python3/dist-packages/pycodestyle.py). - # if: ${{ matrix.os == '24.04' }} - uses: ros-tooling/action-ros-ci@master with: target-ros2-distro: ${{ matrix.distro }} diff --git a/diagnostic_common_diagnostics/CMakeLists.txt b/diagnostic_common_diagnostics/CMakeLists.txt index 5c0f11f6b..e62c86ec0 100644 --- a/diagnostic_common_diagnostics/CMakeLists.txt +++ b/diagnostic_common_diagnostics/CMakeLists.txt @@ -15,6 +15,7 @@ install(PROGRAMS if(BUILD_TESTING) find_package(ament_lint_auto REQUIRED) + find_package(launch_testing_ament_cmake REQUIRED) ament_lint_auto_find_test_dependencies() find_package(ament_cmake_pytest REQUIRED) @@ -22,10 +23,10 @@ if(BUILD_TESTING) test_cpu_monitor test/systemtest/test_cpu_monitor.py TIMEOUT 10) - ament_add_pytest_test( - test_ntp_monitor - test/systemtest/test_ntp_monitor.py - TIMEOUT 10) + add_launch_test( + test/systemtest/test_ntp_monitor_launchtest.py + TARGET ntp_monitor_launchtest + TIMEOUT 20) endif() ament_package() diff --git a/diagnostic_common_diagnostics/diagnostic_common_diagnostics/ntp_monitor.py b/diagnostic_common_diagnostics/diagnostic_common_diagnostics/ntp_monitor.py index 4d52e9e84..9462cebb3 100755 --- a/diagnostic_common_diagnostics/diagnostic_common_diagnostics/ntp_monitor.py +++ b/diagnostic_common_diagnostics/diagnostic_common_diagnostics/ntp_monitor.py @@ -47,13 +47,14 @@ class NTPMonitor(Node): """A diagnostic task that monitors the NTP offset of the system clock.""" - def __init__(self, ntp_hostname, offset=500, self_offset=500, + def __init__(self, ntp_hostname, ntp_port, offset=500, self_offset=500, diag_hostname=None, error_offset=5000000, do_self_test=True): """Initialize the NTPMonitor.""" super().__init__(__class__.__name__) self.ntp_hostname = ntp_hostname + self.ntp_port = ntp_port self.offset = offset self.self_offset = self_offset self.diag_hostname = diag_hostname @@ -67,7 +68,8 @@ def __init__(self, ntp_hostname, offset=500, self_offset=500, self.stat = DIAG.DiagnosticStatus() self.stat.level = DIAG.DiagnosticStatus.OK self.stat.name = 'NTP offset from ' + \ - self.diag_hostname + ' to ' + self.ntp_hostname + self.diag_hostname + ' to ' + self.ntp_hostname + \ + ':' + str(self.ntp_port) self.stat.message = 'OK' self.stat.hardware_id = self.hostname self.stat.values = [] @@ -127,7 +129,10 @@ def add_kv(stat_values, key, value): ntp_client = ntplib.NTPClient() response = None try: - response = ntp_client.request(self.ntp_hostname, version=3) + response = ntp_client.request( + self.ntp_hostname, + port=self.ntp_port, + version=3) except ntplib.NTPException as e: self.get_logger().error(f'NTP Error: {e}') st.level = DIAG.DiagnosticStatus.ERROR @@ -155,11 +160,17 @@ def add_kv(stat_values, key, value): def ntp_monitor_main(argv=sys.argv[1:]): + # filter out ROS args + argv = [a for a in argv if not a.startswith('__') and not a == '--ros-args' and not a == '-r'] + import argparse parser = argparse.ArgumentParser() parser.add_argument('--ntp_hostname', action='store', default='0.pool.ntp.org', type=str) + parser.add_argument('--ntp_port', + action='store', default=123, + type=int) parser.add_argument('--offset-tolerance', dest='offset_tol', action='store', default=500, help='Offset from NTP host [us]', metavar='OFFSET-TOL', @@ -188,7 +199,8 @@ def ntp_monitor_main(argv=sys.argv[1:]): assert offset < error_offset, \ 'Offset tolerance must be less than error offset tolerance' - ntp_monitor = NTPMonitor(args.ntp_hostname, offset, self_offset, + ntp_monitor = NTPMonitor(args.ntp_hostname, args.ntp_port, + offset, self_offset, args.diag_hostname, error_offset, args.do_self_test) diff --git a/diagnostic_common_diagnostics/package.xml b/diagnostic_common_diagnostics/package.xml index de8808d70..8496593a0 100644 --- a/diagnostic_common_diagnostics/package.xml +++ b/diagnostic_common_diagnostics/package.xml @@ -33,6 +33,7 @@ ament_cmake_lint_cmake ament_cmake_pytest + launch_testing_ament_cmake ament_cmake diff --git a/diagnostic_common_diagnostics/test/systemtest/test_ntp_monitor.py b/diagnostic_common_diagnostics/test/systemtest/test_ntp_monitor.py deleted file mode 100644 index 767ed2ac6..000000000 --- a/diagnostic_common_diagnostics/test/systemtest/test_ntp_monitor.py +++ /dev/null @@ -1,130 +0,0 @@ -#!/usr/bin/env python3 -# -*- coding: utf-8 -*- -# Software License Agreement (BSD License) -# -# Copyright (c) 2023, Robert Bosch GmbH -# All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions -# are met: -# -# * Redistributions of source code must retain the above copyright -# notice, this list of conditions and the following disclaimer. -# * Redistributions in binary form must reproduce the above -# copyright notice, this list of conditions and the following -# disclaimer in the documentation and/or other materials provided -# with the distribution. -# * Neither the name of the Willow Garage nor the names of its -# contributors may be used to endorse or promote products derived -# from this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS -# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE -# COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, -# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, -# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; -# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER -# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT -# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN -# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE -# POSSIBILITY OF SUCH DAMAGE. - -import os -import subprocess -import unittest - -import ament_index_python - -from diagnostic_msgs.msg import DiagnosticArray - -import rclpy - -TIMEOUT_MAX_S = 5. - - -class TestNTPMonitor(unittest.TestCase): - - def __init__(self, methodName: str = 'runTest') -> None: - super().__init__(methodName) - rclpy.init() - self.n_msgs_received = 0 - - def setUp(self): - self.n_msgs_received = 0 - n = self._count_msgs(1.) - self.assertEqual(n, 0) - self.subprocess = subprocess.Popen( - [ - os.path.join( - ament_index_python.get_package_prefix( - 'diagnostic_common_diagnostics' - ), - 'lib', - 'diagnostic_common_diagnostics', - 'ntp_monitor.py' - ) - ] - ) - - def tearDown(self): - self.subprocess.kill() - - def _diagnostics_callback(self, msg): - rclpy.logging.get_logger('test_ntp_monitor').info( - f'Received diagnostics message: {msg}' - ) - search_strings = [ - 'NTP offset from', - 'NTP self-offset for' - ] - for search_string in search_strings: - if search_string in ''.join([ - s.name for s in msg.status - ]): - self.n_msgs_received += 1 - - def _count_msgs(self, timeout_s): - self.n_msgs_received = 0 - node = rclpy.create_node('test_ntp_monitor') - rclpy.logging.get_logger('test_ntp_monitor').info( - '_count_msgs' - ) - node.create_subscription( - DiagnosticArray, - 'diagnostics', - self._diagnostics_callback, - 1 - ) - TIME_D_S = .05 - waited_s = 0. - start = node.get_clock().now() - while waited_s < timeout_s and self.n_msgs_received == 0: - rclpy.spin_once(node, timeout_sec=TIME_D_S) - waited_s = (node.get_clock().now() - start).nanoseconds / 1e9 - rclpy.logging.get_logger('test_ntp_monitor').info( - f'received {self.n_msgs_received} messages after {waited_s}s' - ) - node.destroy_node() - return self.n_msgs_received - - def test_publishing(self): - self.assertEqual( - self.subprocess.poll(), - None, - 'NTP monitor subprocess died' - ) - - n = self._count_msgs(TIMEOUT_MAX_S) - - self.assertGreater( - n, - 0, - f'No messages received within {TIMEOUT_MAX_S}s' - ) - - -if __name__ == '__main__': - unittest.main() diff --git a/diagnostic_common_diagnostics/test/systemtest/test_ntp_monitor_launchtest.py b/diagnostic_common_diagnostics/test/systemtest/test_ntp_monitor_launchtest.py index d70470ed2..495c9b684 100644 --- a/diagnostic_common_diagnostics/test/systemtest/test_ntp_monitor_launchtest.py +++ b/diagnostic_common_diagnostics/test/systemtest/test_ntp_monitor_launchtest.py @@ -32,60 +32,84 @@ # ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. -import os +import unittest -import ament_index_python +from diagnostic_msgs.msg import DiagnosticArray import launch -import launch_pytest -from launch_pytest.tools import process as process_tools +import launch_ros import launch_testing +from launch_testing_ros import WaitForTopics + import pytest +import rclpy + -@pytest.fixture -def ntp_monitor_proc(): - # Launch a process to test - return launch.actions.ExecuteProcess( - cmd=[ - os.path.join( - ament_index_python.get_package_prefix( - 'diagnostic_common_diagnostics'), - 'lib', - 'diagnostic_common_diagnostics', - 'ntp_monitor.py' - ), - ], - ), - - -@launch_pytest.fixture -def launch_description(ntp_monitor_proc): +@pytest.mark.launch_test +def generate_test_description(): + """Launch the ntp_monitor node and return a launch description.""" return launch.LaunchDescription([ - ntp_monitor_proc, + launch_ros.actions.Node( + package='diagnostic_common_diagnostics', + executable='ntp_monitor.py', + name='ntp_monitor', + output='screen', + arguments=['--offset-tolerance', '10000', + '--error-offset-tolerance', '20000'] + # 10s, 20s, we are not testing if your clock is correct + ), launch_testing.actions.ReadyToTest() ]) -@pytest.mark.skip(reason='This test is not working yet') -@pytest.mark.launch(fixture=launch_description) -def test_read_stdout(ntp_monitor_proc, launch_context): - """Check if 'ntp_monitor' was found in the stdout.""" - def validate_output(output): - # this function can use assertions to validate the output or return a boolean. - # pytest generates easier to understand failures when assertions are used. - assert output.splitlines() == [ - 'ntp_monitor'], 'process never printed ntp_monitor' - process_tools.assert_output_sync( - launch_context, ntp_monitor_proc, validate_output, timeout=5) - - def validate_output(output): - return output == 'this will never happen' - assert not process_tools.wait_for_output_sync( - launch_context, ntp_monitor_proc, validate_output, timeout=0.1) - yield - # this is executed after launch service shutdown - assert ntp_monitor_proc.return_code == 0 +class TestNtpMonitor(unittest.TestCase): + """Test if the ntp_monitor node is publishing diagnostics.""" + + def __init__(self, methodName: str = 'runTest') -> None: + super().__init__(methodName) + self.received_messages = [] + + def _received_message(self, msg): + self.received_messages.append(msg) + + def _get_min_level(self): + levels = [ + int.from_bytes(status.level, 'little') + for diag in self.received_messages + for status in diag.status] + if len(levels) == 0: + return -1 + return min(levels) + + def test_topic_published(self): + """Test if the ntp_monitor node is publishing diagnostics.""" + with WaitForTopics( + [('/diagnostics', DiagnosticArray)], + timeout=5 + ): + print('Topic found') + + rclpy.init() + test_node = rclpy.create_node('test_node') + test_node.create_subscription( + DiagnosticArray, + '/diagnostics', + self._received_message, + 1 + ) + + while len(self.received_messages) < 10: + rclpy.spin_once(test_node, timeout_sec=1) + if (min_level := self._get_min_level()) == 0: + break + + test_node.destroy_node() + rclpy.shutdown() + print(f'Got {len(self.received_messages)} messages:') + for msg in self.received_messages: + print(msg) + self.assertEqual(min_level, 0)