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.

Sign up for GitHub

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(tier4_localization_rviz_plugin): apply clang-tidy #1608

Merged
merged 9 commits into from
Aug 19, 2022
Merged

refactor(tier4_localization_rviz_plugin): apply clang-tidy #1608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e423c83
refactor(tier4_localization_rviz_plugin): apply clang-tidy
h-ohta Aug 17, 2022
8386171
ci(pre-commit): autofix
pre-commit-ci[bot] Aug 17, 2022
96b963a
refactor: add NOLINT
h-ohta Aug 17, 2022
9278e71
refactor: fix readability-identifier-naming
h-ohta Aug 17, 2022
aaa0bc1
ci(pre-commit): autofix
pre-commit-ci[bot] Aug 17, 2022
5e85599
fix: build error
h-ohta Aug 18, 2022
2191d4e
fix: define copy/move constructor/assign-operator
h-ohta Aug 19, 2022
84127df
ci(pre-commit): autofix
pre-commit-ci[bot] Aug 19, 2022
4f15ed0
chore: add comment
h-ohta Aug 19, 2022
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
15 changes: 6 additions & 9 deletions common/tier4_localization_rviz_plugin/src/pose_history/pose_history_display.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,12 @@ PoseHistory::PoseHistory() : last_stamp_(0, 0, RCL_ROS_TIME)
property_line_width_->setMin(0.0);
}

PoseHistory::~PoseHistory()
{
// Properties are deleted by Qt
}
PoseHistory::~PoseHistory() = default; // Properties are deleted by Qt

void PoseHistory::onInitialize()
{
MFDClass::onInitialize();
lines_.reset(new rviz_rendering::BillboardLine(scene_manager_, scene_node_));
lines_ = std::make_unique<rviz_rendering::BillboardLine>(scene_manager_, scene_node_);
}

void PoseHistory::onEnable() { subscribe(); }
Expand All @@ -65,7 +62,7 @@ void PoseHistory::update(float wall_dt, float ros_dt)
if (!history_.empty()) {
lines_->clear();
if (property_line_view_->getBool()) {
updateLines();
update_lines();
}
}
}
Expand Down Expand Up @@ -95,18 +92,18 @@ void PoseHistory::processMessage(const geometry_msgs::msg::PoseStamped::ConstSha
history_.emplace_back(message);
last_stamp_ = message->header.stamp;

updateHistory();
update_history();
}

void PoseHistory::updateHistory()
void PoseHistory::update_history()
{
const auto buffer_size = static_cast<size_t>(property_buffer_size_->getInt());
while (buffer_size < history_.size()) {
history_.pop_front();
}
}

void PoseHistory::updateLines()
void PoseHistory::update_lines()
{
Ogre::ColourValue color = rviz_common::properties::qtToOgre(property_line_color_->getColor());
color.a = property_line_alpha_->getFloat();
Expand Down
26 changes: 12 additions & 14 deletions common/tier4_localization_rviz_plugin/src/pose_history/pose_history_display.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,13 @@ namespace rviz_rendering
{
class BillboardLine;
} // namespace rviz_rendering
namespace rviz_common
{
namespace properties
namespace rviz_common::properties
{
class ColorProperty;
class FloatProperty;
class IntProperty;
class BoolProperty;
} // namespace properties
} // namespace rviz_common
} // namespace rviz_common::properties

namespace rviz_plugins
{
Expand All @@ -47,23 +44,24 @@ class PoseHistory : public rviz_common::MessageFilterDisplay<geometry_msgs::msg:
public:
PoseHistory();
~PoseHistory() override;
PoseHistory(const PoseHistory &) = delete;
PoseHistory(const PoseHistory &&) = delete;
PoseHistory & operator=(const PoseHistory &) = delete;
PoseHistory & operator=(const PoseHistory &&) = delete;

protected:
void onInitialize() override;
void onEnable() override;
void onDisable() override;
void update(float wall_dt, float ros_dt) override;

private Q_SLOTS:
void subscribe();
void unsubscribe();
void processMessage(const geometry_msgs::msg::PoseStamped::ConstSharedPtr message);

private: // NOLINT for Qt
void updateHistory();
void updateLines();

private:
void subscribe() override;
void unsubscribe() override;
void processMessage(const geometry_msgs::msg::PoseStamped::ConstSharedPtr message) override;
Copy link
Contributor

Choose a reason for hiding this comment

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

@h-ohta
[IMO]
I think every function should be lowerCamelCase or snake_case if you want to change this.
other wise this change is worse than before.

Copy link
Contributor Author

@h-ohta h-ohta Aug 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

These function are defined in parent class. So I can't change it

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand but it looks like strange code style with QT 🤔
if you like this I think it's ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

I resolve this

void update_history();
void update_lines();

std::string target_frame_;
std::deque<geometry_msgs::msg::PoseStamped::ConstSharedPtr> history_;
std::unique_ptr<rviz_rendering::BillboardLine> lines_;
Expand Down