Skip to content

Commit

Permalink
calc inner node level
Browse files Browse the repository at this point in the history
Signed-off-by: Takagi, Isamu <[email protected]>
  • Loading branch information
isamu-takagi committed Mar 14, 2024
1 parent cf6b39b commit 720b066
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 26 deletions.
9 changes: 6 additions & 3 deletions system/diagnostic_graph_aggregator/example/dummy-diags.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ def create_status(name: str):
"external_command_checker: joystick_command",
"external_command_checker: remote_command",
]
rclpy.init()
rclpy.spin(DummyDiagnostics(diags))
rclpy.shutdown()
try:
rclpy.init()
rclpy.spin(DummyDiagnostics(diags))
rclpy.shutdown()
except KeyboardInterrupt:
pass
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<launch>
<arg name="complement"/>
<node pkg="diagnostic_graph_aggregator" exec="converter" name="converter">
<param name="complement_inner_nodes" value="$(var complement)"/>
<param name="complement_tree" value="$(var complement)"/>
</node>
</launch>
54 changes: 34 additions & 20 deletions system/diagnostic_graph_aggregator/src/node/converter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,7 @@

#include "converter.hpp"

#include <memory>
#include <string>
#include <unordered_map>
#include <vector>
#include <algorithm>

namespace diagnostic_graph_aggregator
{
Expand All @@ -42,22 +39,24 @@ std::string parent_path(const std::string & path)
return path.substr(0, path.rfind('/'));

Check warning on line 39 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L39

Added line #L39 was not covered by tests
}

std::vector<std::string> complement_paths(const DiagnosticGraph & graph)
auto create_tree(const DiagnosticGraph & graph)

Check warning on line 42 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L42

Added line #L42 was not covered by tests
{
std::unordered_map<std::string, bool> paths;
std::map<std::string, std::unique_ptr<TreeNode>, std::greater<std::string>> tree;
for (const auto & node : graph.nodes) {
tree.emplace(node.status.name, std::make_unique<TreeNode>(true));

Check warning on line 46 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L45-L46

Added lines #L45 - L46 were not covered by tests
}
for (const auto & node : graph.nodes) {

Check warning on line 48 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L48

Added line #L48 was not covered by tests
std::string path = node.status.name;
paths[path] = false;
while (path = parent_path(path), !path.empty()) {

Check warning on line 50 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L50

Added line #L50 was not covered by tests
if (paths.count(path)) break;
paths[path] = true;
if (tree.count(path)) break;
tree.emplace(path, std::make_unique<TreeNode>(false));

Check warning on line 52 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L52

Added line #L52 was not covered by tests
}
}
std::vector<std::string> result;
for (const auto & [path, flag] : paths) {
if (flag) result.push_back(path);
for (const auto & [path, node] : tree) {
const auto parent = parent_path(path);
node->parent = parent.empty() ? nullptr : tree[parent].get();

Check warning on line 57 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L55-L57

Added lines #L55 - L57 were not covered by tests
}
return result;
return tree;

Check warning on line 59 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L59

Added line #L59 was not covered by tests
}

ConverterNode::ConverterNode() : Node("converter")

Check warning on line 62 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L62

Added line #L62 was not covered by tests
Expand All @@ -69,7 +68,9 @@ ConverterNode::ConverterNode() : Node("converter")
const auto callback = std::bind(&ConverterNode::on_graph, this, _1);
sub_graph_ = create_subscription<DiagnosticGraph>("/diagnostics_graph", qos_graph, callback);
pub_array_ = create_publisher<DiagnosticArray>("/diagnostics_agg", qos_array);

Check warning on line 70 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L70

Added line #L70 was not covered by tests
complement_inner_nodes_ = declare_parameter<bool>("complement_inner_nodes");

initialize_tree_ = false;
complement_tree_ = declare_parameter<bool>("complement_tree");

Check warning on line 73 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L72-L73

Added lines #L72 - L73 were not covered by tests
}

void ConverterNode::on_graph(const DiagnosticGraph::ConstSharedPtr msg)

Check warning on line 76 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L76

Added line #L76 was not covered by tests
Expand All @@ -90,14 +91,27 @@ void ConverterNode::on_graph(const DiagnosticGraph::ConstSharedPtr msg)
}
}

if (complement_inner_nodes_) {
if (!inner_node_names_) {
inner_node_names_ = complement_paths(*msg);
if (complement_tree_ && !initialize_tree_) {
initialize_tree_ = true;
tree_ = create_tree(*msg);

Check warning on line 96 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L94-L96

Added lines #L94 - L96 were not covered by tests
}

if (complement_tree_) {
for (const auto & [path, node] : tree_) {
node->level = DiagnosticStatus::OK;

Check warning on line 101 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L99-L101

Added lines #L99 - L101 were not covered by tests
}
for (const auto & node : msg->nodes) {
tree_[node.status.name]->level = node.status.level;

Check warning on line 104 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L103-L104

Added lines #L103 - L104 were not covered by tests
}
for (const auto & [path, node] : tree_) {
if (!node->parent) continue;
node->parent->level = std::max(node->parent->level, node->level);

Check warning on line 108 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L106-L108

Added lines #L106 - L108 were not covered by tests
}
for (const auto & name : inner_node_names_.value()) {
for (const auto & [path, node] : tree_) {
if (node->leaf) continue;
message.status.emplace_back();
message.status.back().name = name;
message.status.back().level = DiagnosticStatus::STALE;
message.status.back().name = path;
message.status.back().level = node->level;

Check warning on line 114 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.cpp#L110-L114

Added lines #L110 - L114 were not covered by tests
}
}

Check warning on line 117 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Complex Method

ConverterNode::on_graph has a cyclomatic complexity of 15, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.

Check warning on line 117 in system/diagnostic_graph_aggregator/src/node/converter.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Bumpy Road Ahead

ConverterNode::on_graph has 4 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function. The Bumpy Road code smell is a function that contains multiple chunks of nested conditional logic. The deeper the nesting and the more bumps, the lower the code health.
Expand Down
16 changes: 14 additions & 2 deletions system/diagnostic_graph_aggregator/src/node/converter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,32 @@

#include <rclcpp/rclcpp.hpp>

#include <functional>
#include <map> // Use map for sorting keys.
#include <memory>
#include <string>
#include <vector>

namespace diagnostic_graph_aggregator
{

struct TreeNode
{
explicit TreeNode(bool leaf) : leaf(leaf) {}

Check warning on line 33 in system/diagnostic_graph_aggregator/src/node/converter.hpp

View check run for this annotation

Codecov / codecov/patch

system/diagnostic_graph_aggregator/src/node/converter.hpp#L33

Added line #L33 was not covered by tests
bool leaf;
TreeNode * parent;
uint8_t level;
};

class ConverterNode : public rclcpp::Node
{
public:
ConverterNode();

private:
bool complement_inner_nodes_;
std::optional<std::vector<std::string>> inner_node_names_;
bool initialize_tree_;
bool complement_tree_;
std::map<std::string, std::unique_ptr<TreeNode>, std::greater<std::string>> tree_;
rclcpp::Subscription<DiagnosticGraph>::SharedPtr sub_graph_;
rclcpp::Publisher<DiagnosticArray>::SharedPtr pub_array_;
void on_graph(const DiagnosticGraph::ConstSharedPtr msg);
Expand Down

0 comments on commit 720b066

Please sign in to comment.