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

Adding ros_diagnostics publication into rosmon_core. #76

Merged
merged 19 commits into from
May 8, 2019

Conversation

adrienbarral
Copy link
Contributor

Here is my pull request for the feature that add ros diagnostics into rosmon core instead of adding it as a third part node.

Copy link
Owner

@xqms xqms left a comment

Choose a reason for hiding this comment

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

Thanks for the update!

In general, rosmon source code uses tabs for indentation, please adapt. We use tabs for the current indentation level, and spaces for aligning within that indentation level. (Sorry, we don't have a coding style document right now).

What do you think about renaming the "limits" to "warnings"? If someone reads rosmon-memory-limit he/she might think that rosmon actually limits the available memory. I know that name was partly my suggestion, but this just occurred to me...

See the inline stuff for further comments.

rosmon_core/src/launch/bytes_parser.h Outdated Show resolved Hide resolved
rosmon_core/src/launch/bytes_parser.h Outdated Show resolved Hide resolved
rosmon_core/src/launch/bytes_parser.h Outdated Show resolved Hide resolved
rosmon_core/src/launch/bytes_parser.h Outdated Show resolved Hide resolved
rosmon_core/src/launch/launch_config.cpp Outdated Show resolved Hide resolved
rosmon_core/src/main.cpp Outdated Show resolved Hide resolved
@@ -0,0 +1,52 @@
#include <gtest/gtest.h>
Copy link
Owner

Choose a reason for hiding this comment

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

I much prefer catch_ros as the unit testing framework, as this is already used everywhere else in rosmon. If you are not familiar with it, I can convert your testcase later on, no problem.

@xqms xqms mentioned this pull request Apr 1, 2019
Adrien BARRAL added 3 commits April 8, 2019 07:29
…e method signature to not use argument as return value.
Extracting memory string generation into a function, and using the same notation conventions (KiB, MiB ...) as elsewhere in the project.
@xqms
Copy link
Owner

xqms commented Apr 12, 2019

Thanks for the update 👍
I see two things to do before this can be merged:

  • Investigate CI failures - it seems that there is a missing dependency on diagnostics_msgs or similar.
  • Port unit tests to catch_ros. I can look into this, but probably not in the next week.

@adrienbarral
Copy link
Contributor Author

I can't really understand why the CI test fail on jenkins. we have the following error on the ros build farm :

21:28:24 Scanning dependencies of target _run_tests_rosmon_core_rostest_test_basic.test
21:28:24 -- run_tests.py: execute commands
21:28:24   /opt/ros/lunar/share/rostest/cmake/../../../bin/rostest --pkgdir=/tmp/ws/src/rosmon/rosmon_core --package=rosmon_core --results-filename test_basic.xml --results-base-dir "/tmp/ws/test_results" /tmp/ws/src/rosmon/rosmon_core/test/basic.test 
21:28:25 ... logging to /home/buildfarm/.ros/log/rostest-408a64e457cb-1910.log
21:28:26 [ROSUNIT] Outputting test results to /tmp/ws/test_results/rosmon_core/rostest-test_basic.xml
21:28:28 terminate called after throwing an instance of 'fmt::v5::format_error'
21:28:28   what():  precision not allowed for this argument type

On my computer when I try to run this test I have another error !

Traceback (most recent call last):
  File "/home/aba/Projects/MON/workspace/src/rosmon/rosmon_core/test/basic.py", line 20, in <module>
    from rosmon_msgs.msg import State, NodeState
ImportError: No module named msg
          nested_mon: nested_mon exited with status 0
               test1: test1 exited with status 0
               test2: test2 exited with status 0
[Testcase: testrosmon_test] ... FAILURE!

Do you have any idea ?

@xqms
Copy link
Owner

xqms commented Apr 18, 2019

On my system the test fails with exactly the same exception as on the CI server. Maybe you did not build rosmon_msgs or did not source the workspace devel file?

To replicate & debug, you can do:

cd rosmon_core/test
gdb --args ../../../../devel/lib/rosmon_core/rosmon --disable-ui rosmon_core basic.launch

Full backtrace:

(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007ffff53c3801 in __GI_abort () at abort.c:79
#2  0x00007ffff5db68b7 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007ffff5dbca06 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007ffff5dbca41 in std::terminate() () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007ffff5dbcc74 in __cxa_throw () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x00007ffff7bb4045 in fmt::v5::internal::error_handler::on_error (this=<optimized out>, message=0x5555555b6b88 "precision not allowed for this argument type") at /opt/ros/melodic/include/fmt/format-inl.h:821
#7  0x0000555555593fc3 in fmt::v5::basic_parse_context<char, fmt::v5::internal::error_handler>::on_error (message=0x5555555b6b88 "precision not allowed for this argument type", this=<optimized out>) at /home/max/projects/release/devel/.private/rosfmt/include/fmt/core.h:863
#8  fmt::v5::internal::context_base<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, fmt::v5::basic_format_context<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, char>, char>::on_error (
    message=0x5555555b6b88 "precision not allowed for this argument type", this=<optimized out>) at /home/max/projects/release/devel/.private/rosfmt/include/fmt/core.h:953
#9  fmt::v5::internal::specs_handler<fmt::v5::basic_format_context<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, char> >::on_error (this=0x7fffffffba30, message=0x5555555b6b88 "precision not allowed for this argument type")
    at /home/max/projects/release/devel/.private/rosfmt/include/fmt/format.h:1823
#10 fmt::v5::internal::specs_checker<fmt::v5::internal::specs_handler<fmt::v5::basic_format_context<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, char> > >::end_precision (this=0x7fffffffba30) at /home/max/projects/release/devel/.private/rosfmt/include/fmt/format.h:1768
#11 fmt::v5::internal::parse_format_specs<fmt::v5::internal::null_terminating_iterator<char>, fmt::v5::internal::specs_checker<fmt::v5::internal::specs_handler<fmt::v5::basic_format_context<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, char> > >&> (handler=..., it=...)
    at /home/max/projects/release/devel/.private/rosfmt/include/fmt/format.h:2107
#12 fmt::v5::format_handler<fmt::v5::arg_formatter<fmt::v5::back_insert_range<fmt::v5::internal::basic_buffer<char> > >, char, fmt::v5::basic_format_context<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, char> >::on_format_specs (it=..., this=0x7fffffffbac0)
    at /home/max/projects/release/devel/.private/rosfmt/include/fmt/format.h:3328
#13 fmt::v5::internal::parse_format_string<false, char, fmt::v5::format_handler<fmt::v5::arg_formatter<fmt::v5::back_insert_range<fmt::v5::internal::basic_buffer<char> > >, char, fmt::v5::basic_format_context<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, char> >&> (
    format_str=..., handler=...) at /home/max/projects/release/devel/.private/rosfmt/include/fmt/format.h:2188
#14 0x000055555559f7b6 in fmt::v5::vformat_to<fmt::v5::arg_formatter<fmt::v5::back_insert_range<fmt::v5::internal::basic_buffer<char> > >, char, fmt::v5::basic_format_context<std::back_insert_iterator<fmt::v5::internal::basic_buffer<char> >, char> > (args=..., format_str=..., out=...)
    at /home/max/projects/release/devel/.private/rosfmt/include/fmt/format.h:3346
#15 fmt::v5::vformat_to (args=..., format_str=..., buf=Warnung: RTTI symbol not found for class 'fmt::v5::basic_memory_buffer<char, 500ul, std::allocator<char> >'
...) at /home/max/projects/release/devel/.private/rosfmt/include/fmt/format.h:3451
#16 fmt::v5::internal::vformat<char> (format_str=..., args=...) at /home/max/projects/release/devel/.private/rosfmt/include/fmt/format.h:3574
#17 0x00005555555a76d9 in fmt::v5::format<char [5], unsigned long> (format_str=...) at /home/max/projects/release/devel/.private/rosfmt/include/fmt/core.h:1446
#18 rosmon::diagnostics::RosmonToDiagnostic::memoryToString[abi:cxx11](unsigned long) (memory=<optimized out>) at /home/max/projects/release/src/rosmon/rosmon_core/src/diagnostics/rosmon_to_diagnostic.cpp:91
#19 0x00005555555a7cbf in rosmon::diagnostics::RosmonToDiagnostic::updateDiagnostics (this=0x555555ccbd60, state=...) at /home/max/projects/release/src/rosmon/rosmon_core/src/diagnostics/rosmon_to_diagnostic.cpp:41
#20 0x00005555555ae0e0 in rosmon::ROSInterface::update (this=0x7fffffffceb0) at /home/max/projects/release/src/rosmon/rosmon_core/src/ros_interface.cpp:38
#21 0x00007ffff78c4e17 in ros::TimerManager<ros::WallTime, ros::WallDuration, ros::WallTimerEvent>::TimerQueueCallback::call() () from /opt/ros/melodic/lib/libroscpp.so
#22 0x00007ffff78f8f1c in ros::CallbackQueue::callOneCB(ros::CallbackQueue::TLS*) () from /opt/ros/melodic/lib/libroscpp.so
#23 0x00007ffff78fa30b in ros::CallbackQueue::callAvailable(ros::WallDuration) () from /opt/ros/melodic/lib/libroscpp.so
#24 0x00007ffff793a4ce in ros::spinOnce() () from /opt/ros/melodic/lib/libroscpp.so
#25 0x0000555555581cbf in main (argc=<optimized out>, argv=<optimized out>) at /home/max/projects/release/src/rosmon/rosmon_core/src/main.cpp:468

It's failing here:

#18 rosmon::diagnostics::RosmonToDiagnostic::memoryToString[abi:cxx11](unsigned long) (memory=<optimized out>) at /home/max/projects/release/src/rosmon/rosmon_core/src/diagnostics/rosmon_to_diagnostic.cpp:91
91              return fmt::format("{:.2f} MiB", memory / static_cast<uint64_t>(1<<20));

I guess the .2f modifier only works for float, but memory / ... is an integer.

@xqms
Copy link
Owner

xqms commented May 8, 2019

Okay, I did the port to catch_ros and did some modifications to match coding style.

Open todos:

  • We display memory sizes as GiB, MiB (i.e. with 1024 multipliers) and parse them as MB (1000 multiplier). Maybe we should support both in the parser?
  • Test this in our production setups (that's mainly a todo for me)

@xqms
Copy link
Owner

xqms commented May 8, 2019

Seems to work in the workspaces I have for testing. I raised the default memory & CPU limits though, many nodes use more than 50MB memory and that's not a problem on most platforms.

I'll wait for CI results and then we can merge :-)

@xqms
Copy link
Owner

xqms commented May 8, 2019

Seems to work in the workspaces I use for testing. I raised the default memory & CPU limits though, there are were many nodes using more than 50MB RAM where I would say that is not a problem.

Let's go ahead and merge this! :-)
Since this is kind of an additional feature, I'm fine with testing it more in master.

@xqms
Copy link
Owner

xqms commented May 8, 2019

huh, strange issue with the comment timestamps, sorry for the double post. Somehow I'm able to write from the future...

@xqms xqms merged commit 0af2f78 into xqms:master May 8, 2019
gauthamm added a commit to rapyuta-robotics/rosmon that referenced this pull request Jul 15, 2021
* core: launch_config: parse attributes in parent context

and make it mutable (required for $(anon))

* core: launch_config: handle $(anon) in ParseContext, see xqms#100

* core: test: node_utils: more debug output

* core: test: test_subst: test for nested $(anon), see xqms#100

* fix two typos in the README

* rosmon_core: fix a lot of whitespace issues introduced in xqms#76

* rosmon_core: option to obey output="XY" tags (see xqms#108)

Co-authored-by: Roger Pi <u1926981@campus.udg.edu>

* rosmon_core: unit test for output-attr parsing, refs xqms#108

* fix xqms#110

* rosmon_core: do not print node prefix in --disable-ui mode

--disable-ui is mostly used with ncurses-like nodes that have problems
with such additional outputs.

* rosmon_core: node_monitor: flush out remaining node output after node exit

* rosmon_core: new rosmon-disable-ui attribute for problematic launch files

* core: launch_config: accept empty yaml node

* rosmon_core: LaunchConfig warnings to stderr & make them captureable

* rosmon_core: test: unit tests for the warnings we trigger

* rosmon_core: Add YAML merge key parsing functionality.

* rosmon_core: Adding fix for rosparam parsing to equivilate with undocumented behavior of roslaunch

* rosmon_core: test: unit tests for merge keys feature

* rosmon_core: ui: UI DrawStatus optimized to only refresh on events.

* core: launch: add some comments & fix whitespace

* rosmon_core: depend on python2/3 conditioned on ROS_PYTHON_VERSION

* core: launch: add comments

* core: ui: just redraw on every keystroke

* core: launch: shift some node attributes into context

See xqms#116

* rosmon_core: parse scope attributes also for launch, include, group tags

* core: launch: make "enable-coredumps" a scoped attribute

* core: test: add test for scoped attributes on include tags

* core: monitor: support for systemd-coredump

* core: monitor: delete working directories from two runs ago

* capture stderr separately and correctly do output=log

* roscore: test: Fixing unit tests for stdout_stderr mod

* Adding alt. stderr output filter

* CHANGELOG

* CHANGELOG

* 2.3.0

* rosmon_core: cmake: request specific boost_python3 version

* rosmon_core: test: adapt test_node to python 3

* CHANGELOG

* 2.3.1

* rosmon_core: *really* fix search for boost_python for python 3

* rosmon_core: cmake: increase minimum version to address policy warnings

* CHANGELOG

* 2.3.2

* rosmon_core: cmake: older cmake versions don't have GREATER_EQUAL

* all: increase cmake minimum version to 3.4

* README: add installation hint for rosmon_core

* core: launch_config: loadYAMLParams: don't scope keys with leading slash

See xqms#130 for bug report. Basically, roslaunch respects leading slashes
during key resolution, we don't. We don't know what's better, but we have
to match whatever roslaunch is doing, as always.

* core: test: xml: rosparam: test leading slashes in keys

* core: consolidate floating point types used for stats

Fixes problems identified with -Wfloat-conversion

* core: ui: explicit float casts

Fixes problems identified with -Wfloat-conversion.

* rqt: make float/int casts explicit

Identified with -Wfloat-conversion.

* rosmon_core: env-hooks: follow symbolic links during file name suggestion

This fixes bash completion for colcon linked install spaces.

* core: substitution_python: catch by reference

Fixes a compiler warning on Ubuntu Focal.

* core: ui: display & search full name including namespace

* core: ui: use full name everywhere

* Start all and stop all options are added
F6 -> start all
F7 -> stop all

* core: launch: bytes_parser: add missing include

* Added option --stdout-print-source to print out node name when --disable-ui is used. Addresses xqms#139.

* Removed stdout-print-source option and print node name by default when --disable-ui is used. Right justify node names to the max node name length.

* core: fix compilation on Kinetic (boost::placeholders came later)

* core: monitor: do not leak stderr file descriptors

* core: launch_config: add warning for type= attrs on <param> tags on Kinetic

See xqms#138.

* update warning

* core: launch_config: strip off leading/trailing whitespace of params

* core: add utility to precisely dump ROS parameters

This is useful to analyze differences between roslaunch and rosmon in
parameter loading.

* core: launch_config: use double precision for auto type parameters

Previously we used floats, but XmlRpc stores doubles, so we should
make use of the full precision.

* core: launch_config: convert tabs + newlines in parameters

* core: test: add unit test for all sorts of whitespace in parameters

* core: test: basic.launch: make it work with roslaunch

* core: test: basic.py: adapt to fixed whitespace behavior

* core: fix aliasing problem using std::memcpy

* Resolves xqms#145 by providing a number of restarts threshold per node via ROS-launch arguments

* core: fix signed/unsigned comparison warning

* core: install tests (necessary for testing with colcon)

* core: monitor: use a second PTY for stderr, see xqms#147

* Inidicate when a node is actually Idle (e.g. stopped)

* Add missing dependency on libboost-pyhton-dev

Co-authored-by: Max Schwarz <max.schwarz@ais.uni-bonn.de>
Co-authored-by: Max Schwarz <max.schwarz@online.de>
Co-authored-by: Roger Pi <u1926981@campus.udg.edu>
Co-authored-by: Kazuhiro Hiratsuka <papino0319@gmail.com>
Co-authored-by: marco-tranzatto <marcot@ethz.ch>
Co-authored-by: Carl Colena <carl.colena@gmail.com>
Co-authored-by: Kutay YILMAZ <ktyylmz035@gmail.com>
Co-authored-by: Steve Golton <stevegolton@gmail.com>
Co-authored-by: mcfurry <f.b.f.schoenmakers@gmail.com>
Co-authored-by: Ferry Schoenmakers <ferry.schoenmakers@nobleo.nl>
Co-authored-by: Tim Clephas <tim.clephas@nobleo.nl>
gauthamm pushed a commit to rapyuta-robotics/rosmon that referenced this pull request Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants