Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

VladyslavHrynchak200204
Copy link

@VladyslavHrynchak200204 VladyslavHrynchak200204 commented Oct 15, 2024

Reason for change:

https://github.com/ros/diagnostics/ now has a CPU and RAM monitor built-in. Please

create a new branch in our fork and update it to latest ros2 from upstream (contact me in case of doubts about conflicts)
replace the CPU and RAM monitor we currently use from system_monitor with the ones from diagnostics
compare their statistics on an AMR (ensure that they give similar values, if not, document how they change).
remove system_monitor from AMR.

Changes:

Added compatibility for CPU and RAM monitor from upstream
Changes for https://github.com/logivations/deep_cv/pull/8482

Result:

image

image

image

ct2034 and others added 30 commits January 18, 2024 23:03
… marked discard_stale, or when all of their items are STALE. (ros#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 <andrew.c.symington@gmail.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Christian Henkel <christian.henkel2@de.bosch.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
* 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 <richard.van.de.ketterij@nobleo.nl>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
This reverts commit 5077016.
* runs on container
* all on ubuntu latest
* pydocstyle fix is obsolete
* no uncrustify for noble

---------

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
- using a launchtest

---------

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
* testing with ubuntu ntp server
* message makes a little more sense with abs
* bigger offsets allowed
* organizing imports with isort profile `google`

---------

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
* NTP monitor improvements
* commit to run tests again
* bring back double callback
* remove whiteline

---------

Co-authored-by: Angsa Deployment Team <team@angsa-robotics.com>
* refactor(ram_monitor): ros2 port
* Update diagnostic_common_diagnostics/diagnostic_common_diagnostics/ram_monitor.py
* Apply suggestions from code review
* Update diagnostic_common_diagnostics/README.md
* fstrings
* python3

---------

Co-authored-by: Christian Henkel <6976069+ct2034@users.noreply.github.com>
* Fix usage of rclcpp::ok with a non-default context

The current implementation calls `rclcpp::ok` without any arguments,
which amounts to verifying that the global default context is valid. In
the case where a node is added to a custom context, and the global
context is not used, `rclcpp::ok` without any arguments will always
return `false` since the global context has never been initialized. To
fix it, pass to rclcpp the context that's available via the node's base
interface.

* Add a test for custom context
…egradation (ros#324)

* Aggregator: publish diagnostics_toplevel_state immediately on every degradation
* Update docs
* Re-use the publishData function such that also the actual diagnostics are available
* Expand test with more degradation cases
* Add add_analyzer functionality
* Add copyright notice and license, remove unused includes, re-order includes correctly
* Increase clarity of prefix_ by renaming it to analyzers_ns_
* Add add_analyzer functionality
* Fix bug where base_path is not reset correctly
* Make the parameter forwarding condition more generic, fix the default service namespace from diagnostics_agg to analyzers
* Add an add_analyzer example to the diagnostic_aggregator
* Update the relevant READMEs
* Fix linter errors
* Add test for add_analyzer at runtime, remove unnecessary ros info logger, remove unnecessary hardcoded namespace from yaml files
* Remove the now redundant analyzers_ns_
* Change the copyright of add_analyzer, forgot to update it to Nobleo after copying the notice
thanks to https://github.com/reinzor

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
A new clock was created for every diagnosed publisher. When running with
'use_sim_time', the /clock time was not respected.
* Adoptin Ci changes similar to jazzy

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>

* also ignoring pep257 here ...

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>

---------

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
* Port hd_monitor.py
* Adapt documentation and CMakeList to new hd_monitor.py
* Fix execution flag of hd_monitor
* Add launch test for hd_monitor
* Implement low and crit parameters in hd_monitor
* Improve hd_monitor code quality
ct2034 and others added 8 commits July 17, 2024 14:51
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
…ix usage of rclcpp::ok with a non-default context (ros#352)  (ros#390)

* Minimize header includes by moving impl to .cpp files (ros#331)

* Minimize header includes by moving impl to .cpp files
* Make sure to build a shared library

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>

* Fix usage of rclcpp::ok with a non-default context (ros#352)

* Fix usage of rclcpp::ok with a non-default context

The current implementation calls `rclcpp::ok` without any arguments,
which amounts to verifying that the global default context is valid. In
the case where a node is added to a custom context, and the global
context is not used, `rclcpp::ok` without any arguments will always
return `false` since the global context has never been initialized. To
fix it, pass to rclcpp the context that's available via the node's base
interface.

* Add a test for custom context

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>

---------

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Ramon Wijnands <ramon.wijnands007@gmail.com>
Co-authored-by: Hervé Audren <101862279+haudren-woven@users.noreply.github.com>
Without this, downstream packages have missing symbols

Relates to: ros#387

(cherry picked from commit d8fef69)

Co-authored-by: Ramon Wijnands <ramon.wijnands@nobleo.nl>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
…R-diagnostics-Use-CPU-and-RAM-monitor-from-diagnostics
tests_require=["pytest"],
entry_points={
'console_scripts': [
'cpu_monitor = diagnostic_common_diagnostics.cpu_monitor:main',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added setup.py to diagnostics_common_diagnostics package to easily call cpu_monitor and ram_monitor as executable

@@ -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)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved conflicts: added analyzer to diagnostic_aggregator/CMakeLists.txt

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolve conflicts: added rclcpp::NodeOptions options to Aggregator::Aggregator constructor

{ // lock the mutex while analyzer_group_ and other_analyzer_ are being updated
std::lock_guard<std::mutex> lock(mutex_);
n_ = std::shared_ptr<rclcpp::Node>(this, [](rclcpp::Node *) {});
analyzer_group_ = std::make_unique<AnalyzerGroup>();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolve conflicts: added lock_guard

# Create the node
hostname = socket.gethostname()
node = Node(f'cpu_monitor_{hostname.replace("-", "_")}')
node = Node('cpu_monitor')

# Declare and get parameters
node.declare_parameter('warning_percentage', 90)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added get_cpu_diagnostics_node for easy launch from python_launch.py and changed node name to cpu_monitor instead of cpu_monitor_{hostname.replace("-", "_") not to be depended of hostname in yaml file

rclpy.init()
node = rclpy.create_node(f'ram_monitor_{hostname.replace("-", "_")}')

node = rclpy.create_node('ram_monitor')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same changed for ram_monitor as listed above

@VladyslavHrynchak200204 VladyslavHrynchak200204 merged commit bbb18d0 into ros2 Oct 22, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.