Add diagnostics monitor nodes #467
Conversation
| class Publisher : public rclcpp::Node { | ||
| public: | ||
| Publisher() : Node("heartbeat_publisher"), diagnostic_updater_{this} { | ||
| Publisher() : Node("heartbeat_publisher"), diagnostic_updater_(this) { |
There was a problem hiding this comment.
This change seems to be unrelated to this PR. Please revert.
There was a problem hiding this comment.
The CI fails without this change. Looks like the log expired, but it's a brace initialization problem.
Between versions 9f402787ea2c9b3dd4d7e51a9986810e8a3400ba and 207f0c0f1f8e9bd1842a07d90459aa9a2b670c23 of ros/diagnostics, they moved one of the Updater constructors from a header file to a source file. That constructor is then used by the templated constructor. Since we're using the templated constructor, brace initialization requires that its definition is completely visible from the header file. Since it's delegating to another constructor that's now defined in the source file, brace initialization won't work.
I should have put the reason for this in the PR description. Added!
There was a problem hiding this comment.
To be more clear: they moved the definition of this constructor to a source file.
It's used by the templated constructor on line 371.
| ) | ||
|
|
||
| ros2_py_binary( | ||
| name = "hd_monitor_node", |
There was a problem hiding this comment.
It stands for "hard drive". hd_monitor_node has an unfortunate name, but I figure we ought to keep the naming consistent with the upstream stuff. To help out on our end, I added a comment clarifying this is "hard drive".
| ], | ||
| ) | ||
|
|
||
| ros2_py_binary( |
There was a problem hiding this comment.
Sorry for the delay. Why do you need set_up_ament = True? How is this supposed to work in practice? Just launch the nodes?
There was a problem hiding this comment.
I suppose we don't need it. Change to False. Thanks!
repositories/repositories.bzl
Outdated
| urls = ["https://github.com/ros/diagnostics/archive/9f402787ea2c9b3dd4d7e51a9986810e8a3400ba.zip"], | ||
| sha256 = "66b00679cc5404fbe41c4ee486382235f80900bc21bd7937ba25ffca469ec5d4", | ||
| strip_prefix = "diagnostics-207f0c0f1f8e9bd1842a07d90459aa9a2b670c23", | ||
| urls = ["https://github.com/ros/diagnostics/archive/207f0c0f1f8e9bd1842a07d90459aa9a2b670c23.zip"], |
There was a problem hiding this comment.
Hi, I am very sorry for such a late reply. Can we use https://github.com/ros/diagnostics/releases/tag/4.4.6 tag here?
There was a problem hiding this comment.
Good idea! Done
| name = "hd_monitor_node", | ||
| srcs = ["diagnostic_common_diagnostics/diagnostic_common_diagnostics/hd_monitor.py"], | ||
| main = "diagnostic_common_diagnostics/diagnostic_common_diagnostics/hd_monitor.py", | ||
| set_up_ament = False, |
There was a problem hiding this comment.
Why do you need set_up_ament = False here and below? If not necessary, please remove.
2 participants
Add diagnostics monitor nodes from
diagnostic_common_diagnostics.Includes the following updates:
cpu_monitor_node,hd_monitor_node,ntp_monitor_node,ram_monitor_node, andsensors_monitor_nodentplibto the pip requirements filesros/diagnosticsrepo to207f0c0f1f8e9bd1842a07d90459aa9a2b670c23cpp_diagnostic_updaterto use new source files that were not in the previous version ofros/diagnosticsdiagnostic_updater_, since brace initialization will not work with the templated constructor anymore (a delegated constructor's definition was moved from a header to a source file)