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

refactor(control_evaluator): use class naming standard and use remapped param name #7782

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion evaluator/autoware_control_evaluator/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ament_auto_add_library(control_evaluator_node SHARED
)

rclcpp_components_register_node(control_evaluator_node
PLUGIN "control_diagnostics::controlEvaluatorNode"
PLUGIN "control_diagnostics::ControlEvaluatorNode"
EXECUTABLE control_evaluator
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ using geometry_msgs::msg::AccelWithCovarianceStamped;
/**
* @brief Node for control evaluation
*/
class controlEvaluatorNode : public rclcpp::Node
class ControlEvaluatorNode : public rclcpp::Node
{
public:
explicit controlEvaluatorNode(const rclcpp::NodeOptions & node_options);
explicit ControlEvaluatorNode(const rclcpp::NodeOptions & node_options);
void removeOldDiagnostics(const rclcpp::Time & stamp);
void removeDiagnosticsByName(const std::string & name);
void addDiagnostic(const DiagnosticStatus & diag, const rclcpp::Time & stamp);
Expand Down Expand Up @@ -83,7 +83,7 @@ class controlEvaluatorNode : public rclcpp::Node
autoware::universe_utils::InterProcessPollingSubscriber<Odometry> odometry_sub_{
this, "~/input/odometry"};
autoware::universe_utils::InterProcessPollingSubscriber<AccelWithCovarianceStamped> accel_sub_{
this, "/localization/acceleration"};
this, "~/input/acceleration"};
autoware::universe_utils::InterProcessPollingSubscriber<Trajectory> traj_sub_{
this, "~/input/trajectory"};
autoware::universe_utils::InterProcessPollingSubscriber<LaneletRoute> route_subscriber_{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2024 Tier IV, Inc.

Check notice on line 1 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ Getting better: Code Duplication

reduced similar code in: controlEvaluatorNode::generateLateralDeviationDiagnosticStatus,controlEvaluatorNode::generateYawDeviationDiagnosticStatus. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

Check notice on line 1 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ No longer an issue: Complex Conditional

controlEvaluatorNode::onTimer no longer has a complex conditional. A complex conditional is an expression inside a branch (e.g. if, for, while) which consists of multiple, logical operators such as AND/OR. The more logical operators in an expression, the more severe the code smell.

Check notice on line 1 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

ℹ New issue: Complex Conditional

ControlEvaluatorNode::onTimer has 1 complex conditionals with 2 branches, threshold = 2. A complex conditional is an expression inside a branch (e.g. if, for, while) which consists of multiple, logical operators such as AND/OR. The more logical operators in an expression, the more severe the code smell.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -25,23 +25,23 @@

namespace control_diagnostics
{
controlEvaluatorNode::controlEvaluatorNode(const rclcpp::NodeOptions & node_options)
ControlEvaluatorNode::ControlEvaluatorNode(const rclcpp::NodeOptions & node_options)

Check warning on line 28 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L28

Added line #L28 was not covered by tests
: Node("control_evaluator", node_options)
{
using std::placeholders::_1;
control_diag_sub_ = create_subscription<DiagnosticArray>(
"~/input/diagnostics", 1, std::bind(&controlEvaluatorNode::onDiagnostics, this, _1));
"~/input/diagnostics", 1, std::bind(&ControlEvaluatorNode::onDiagnostics, this, _1));

Check warning on line 33 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L33

Added line #L33 was not covered by tests

// Publisher
metrics_pub_ = create_publisher<DiagnosticArray>("~/metrics", 1);

// Timer callback to publish evaluator diagnostics
using namespace std::literals::chrono_literals;
timer_ =
rclcpp::create_timer(this, get_clock(), 100ms, std::bind(&controlEvaluatorNode::onTimer, this));
rclcpp::create_timer(this, get_clock(), 100ms, std::bind(&ControlEvaluatorNode::onTimer, this));

Check warning on line 41 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L41

Added line #L41 was not covered by tests
}

void controlEvaluatorNode::getRouteData()
void ControlEvaluatorNode::getRouteData()

Check warning on line 44 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L44

Added line #L44 was not covered by tests
{
// route
{
Expand All @@ -64,7 +64,7 @@
}
}

void controlEvaluatorNode::removeOldDiagnostics(const rclcpp::Time & stamp)
void ControlEvaluatorNode::removeOldDiagnostics(const rclcpp::Time & stamp)

Check warning on line 67 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L67

Added line #L67 was not covered by tests
{
constexpr double KEEP_TIME = 1.0;
diag_queue_.erase(
Expand All @@ -76,7 +76,7 @@
diag_queue_.end());
}

void controlEvaluatorNode::removeDiagnosticsByName(const std::string & name)
void ControlEvaluatorNode::removeDiagnosticsByName(const std::string & name)

Check warning on line 79 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L79

Added line #L79 was not covered by tests
{
diag_queue_.erase(
std::remove_if(
Expand All @@ -87,13 +87,13 @@
diag_queue_.end());
}

void controlEvaluatorNode::addDiagnostic(
void ControlEvaluatorNode::addDiagnostic(

Check warning on line 90 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L90

Added line #L90 was not covered by tests
const diagnostic_msgs::msg::DiagnosticStatus & diag, const rclcpp::Time & stamp)
{
diag_queue_.push_back(std::make_pair(diag, stamp));
}

void controlEvaluatorNode::updateDiagnosticQueue(
void ControlEvaluatorNode::updateDiagnosticQueue(

Check warning on line 96 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L96

Added line #L96 was not covered by tests
const DiagnosticArray & input_diagnostics, const std::string & function,
const rclcpp::Time & stamp)
{
Expand All @@ -110,15 +110,15 @@
removeOldDiagnostics(stamp);
}

void controlEvaluatorNode::onDiagnostics(const DiagnosticArray::ConstSharedPtr diag_msg)
void ControlEvaluatorNode::onDiagnostics(const DiagnosticArray::ConstSharedPtr diag_msg)

Check warning on line 113 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L113

Added line #L113 was not covered by tests
{
// add target diagnostics to the queue and remove old ones
for (const auto & function : target_functions_) {
updateDiagnosticQueue(*diag_msg, function, now());
}
}

DiagnosticStatus controlEvaluatorNode::generateAEBDiagnosticStatus(const DiagnosticStatus & diag)
DiagnosticStatus ControlEvaluatorNode::generateAEBDiagnosticStatus(const DiagnosticStatus & diag)

Check warning on line 121 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L121

Added line #L121 was not covered by tests
{
DiagnosticStatus status;
status.level = status.OK;
Expand All @@ -131,7 +131,7 @@
return status;
}

DiagnosticStatus controlEvaluatorNode::generateLaneletDiagnosticStatus(const Pose & ego_pose) const
DiagnosticStatus ControlEvaluatorNode::generateLaneletDiagnosticStatus(const Pose & ego_pose) const

Check warning on line 134 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L134

Added line #L134 was not covered by tests
{
const auto current_lanelets = [&]() {
lanelet::ConstLanelet closest_route_lanelet;
Expand Down Expand Up @@ -162,7 +162,7 @@
return status;
}

DiagnosticStatus controlEvaluatorNode::generateKinematicStateDiagnosticStatus(
DiagnosticStatus ControlEvaluatorNode::generateKinematicStateDiagnosticStatus(

Check warning on line 165 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L165

Added line #L165 was not covered by tests
const Odometry & odom, const AccelWithCovarianceStamped & accel_stamped)
{
DiagnosticStatus status;
Expand Down Expand Up @@ -198,7 +198,7 @@
return status;
}

DiagnosticStatus controlEvaluatorNode::generateLateralDeviationDiagnosticStatus(
DiagnosticStatus ControlEvaluatorNode::generateLateralDeviationDiagnosticStatus(

Check notice on line 201 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

ℹ Getting worse: Code Duplication

introduced similar code in: ControlEvaluatorNode::generateLateralDeviationDiagnosticStatus,ControlEvaluatorNode::generateYawDeviationDiagnosticStatus. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

Check warning on line 201 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L201

Added line #L201 was not covered by tests
const Trajectory & traj, const Point & ego_point)
{
const double lateral_deviation = metrics::calcLateralDeviation(traj, ego_point);
Expand All @@ -214,7 +214,7 @@
return status;
}

DiagnosticStatus controlEvaluatorNode::generateYawDeviationDiagnosticStatus(
DiagnosticStatus ControlEvaluatorNode::generateYawDeviationDiagnosticStatus(

Check warning on line 217 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L217

Added line #L217 was not covered by tests
const Trajectory & traj, const Pose & ego_pose)
{
const double yaw_deviation = metrics::calcYawDeviation(traj, ego_pose);
Expand All @@ -230,7 +230,7 @@
return status;
}

void controlEvaluatorNode::onTimer()
void ControlEvaluatorNode::onTimer()

Check notice on line 233 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ No longer an issue: Complex Method

controlEvaluatorNode::onTimer is no longer above the threshold for cyclomatic complexity. 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 notice on line 233 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

ℹ New issue: Complex Method

ControlEvaluatorNode::onTimer has a cyclomatic complexity of 12, 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 233 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L233

Added line #L233 was not covered by tests
{
DiagnosticArray metrics_msg;
const auto traj = traj_sub_.takeData();
Expand Down Expand Up @@ -278,4 +278,4 @@
} // namespace control_diagnostics

#include "rclcpp_components/register_node_macro.hpp"
RCLCPP_COMPONENTS_REGISTER_NODE(control_diagnostics::controlEvaluatorNode)
RCLCPP_COMPONENTS_REGISTER_NODE(control_diagnostics::ControlEvaluatorNode)

Check warning on line 281 in evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp

View check run for this annotation

Codecov / codecov/patch

evaluator/autoware_control_evaluator/src/control_evaluator_node.cpp#L281

Added line #L281 was not covered by tests
2 changes: 1 addition & 1 deletion launch/tier4_control_launch/launch/control.launch.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ def launch_setup(context, *args, **kwargs):
# control evaluator
control_evaluator_component = ComposableNode(
package="autoware_control_evaluator",
plugin="control_diagnostics::controlEvaluatorNode",
plugin="control_diagnostics::ControlEvaluatorNode",
name="control_evaluator",
remappings=[
("~/input/diagnostics", "/diagnostics"),
Expand Down
Loading