perf: use single TF listener & add singleton #5
Conversation
Signed-off-by: Amadeusz Szymko <[email protected]>
Signed-off-by: Amadeusz Szymko <[email protected]>
Signed-off-by: Amadeusz Szymko <[email protected]>
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| Eigen::Matrix4f eigen_base_to_lidar_top_; | ||
| Eigen::Matrix4f eigen_base_to_lidar_right_; | ||
| std::unique_ptr<sensor_msgs::msg::PointCloud2> cloud_in_; | ||
| std::unique_ptr<sensor_msgs::msg::PointCloud2> cloud_in_{nullptr}; | ||
| double precision_; |
There was a problem hiding this comment.
constructor does not initialize these fields: precision_
| double precision_; | |
| double precision_{}; |
| double precision_; | ||
| rclcpp::Time time_; | ||
| rclcpp::Duration timeout_ = rclcpp::Duration::from_seconds(1); | ||
|
|
||
| geometry_msgs::msg::TransformStamped generateTransformMsg( |
There was a problem hiding this comment.
invalid case style for function generateTransformMsg
| geometry_msgs::msg::TransformStamped generateTransformMsg( | |
| geometry_msgs::msg::TransformStamped generate_transform_msg( |
|
|
||
| tf_map_to_base_ = generateTransformMsg( |
There was a problem hiding this comment.
invalid case style for function generateTransformMsg
| tf_map_to_base_ = generateTransformMsg( | |
| tf_map_to_base_ = generate_transform_msg( |
|
|
||
| tf_map_to_base_ = generateTransformMsg( | ||
| 10, 100'000'000, "map", "base_link", 120.0, 240.0, 1.0, 0.0, 0.0, 0.0, 1.0); | ||
| tf_base_to_lidar_top_ = generateTransformMsg( |
There was a problem hiding this comment.
invalid case style for function generateTransformMsg
| tf_base_to_lidar_top_ = generateTransformMsg( | |
| tf_base_to_lidar_top_ = generate_transform_msg( |
|
|
||
| tf_map_to_base_ = generateTransformMsg( | ||
| 10, 100'000'000, "map", "base_link", 120.0, 240.0, 1.0, 0.0, 0.0, 0.0, 1.0); | ||
| tf_base_to_lidar_top_ = generateTransformMsg( | ||
| 10, 100'000'000, "base_link", "lidar_top", 0.690, 0.000, 2.100, -0.007, -0.007, 0.692, 0.722); | ||
| tf_base_to_lidar_right_ = generateTransformMsg( |
There was a problem hiding this comment.
invalid case style for function generateTransformMsg
| tf_base_to_lidar_right_ = generateTransformMsg( | |
| tf_base_to_lidar_right_ = generate_transform_msg( |
| @@ -69,23 +72,45 @@ | |||
| return tf_msg; | |||
| } | |||
|
|
|||
| void broadcastDynamicTf(geometry_msgs::msg::TransformStamped transform, uint32_t seconds = 1) | |||
There was a problem hiding this comment.
invalid case style for function broadcastDynamicTf
| void broadcastDynamicTf(geometry_msgs::msg::TransformStamped transform, uint32_t seconds = 1) | |
| void broadcast_dynamic_tf(geometry_msgs::msg::TransformStamped transform, uint32_t seconds = 1) |
| static_tf_broadcaster_->sendTransform(tf_base_to_lidar_right_); | ||
|
|
||
| std::future<void> future = | ||
| std::async(std::launch::async, [this]() { broadcastDynamicTf(tf_map_to_base_); }); |
There was a problem hiding this comment.
invalid case style for function broadcastDynamicTf
| std::async(std::launch::async, [this]() { broadcastDynamicTf(tf_map_to_base_); }); | |
| std::async(std::launch::async, [this]() { broadcast_dynamic_tf(tf_map_to_base_); }); |
| ASSERT_TRUE(eigen_transform.has_value()); | ||
| EXPECT_TRUE(eigen_transform.value().isApprox(eigen_base_to_lidar_top_, precision_)); | ||
|
|
||
| std::future<void> future = | ||
| std::async(std::launch::async, [this]() { broadcastDynamicTf(tf_map_to_base_); }); |
There was a problem hiding this comment.
invalid case style for function broadcastDynamicTf
| std::async(std::launch::async, [this]() { broadcastDynamicTf(tf_map_to_base_); }); | |
| std::async(std::launch::async, [this]() { broadcast_dynamic_tf(tf_map_to_base_); }); |
| @@ -69,23 +72,45 @@ | |||
| return tf_msg; | |||
| } | |||
|
|
|||
| void broadcastDynamicTf(geometry_msgs::msg::TransformStamped transform, uint32_t seconds = 1) | |||
There was a problem hiding this comment.
the parameter transform is copied for each invocation but only used as a const reference; consider making it a const reference
| void broadcastDynamicTf(geometry_msgs::msg::TransformStamped transform, uint32_t seconds = 1) | |
| void broadcastDynamicTf(const geometry_msgs::msg::TransformStamped& transform, uint32_t seconds = 1) |
| #include <atomic> | ||
| #include <cstdint> | ||
| #include <future> | ||
|
|
There was a problem hiding this comment.
pass by value and use std::move
| #include <utility> | |
|
|
||
| ManagedTransformBufferProvider::ManagedTransformBufferProvider( | ||
| rclcpp::Clock::SharedPtr clock, tf2::Duration cache_time) | ||
| : clock_(clock) |
There was a problem hiding this comment.
pass by value and use std::move
| : clock_(clock) | |
| : clock_(std::move(clock)) |
| } | ||
| } | ||
|
|
||
| TraverseResult ManagedTransformBufferProvider::traverseTree( |
There was a problem hiding this comment.
function traverseTree has cognitive complexity of 38 (threshold 25)
| { | ||
| std::atomic<bool> timeout_reached{false}; | ||
|
|
||
| auto framesToRoot = [this]( |
There was a problem hiding this comment.
invalid case style for variable framesToRoot
| auto framesToRoot = [this]( | |
| auto frames_to_root = [this]( |
| return true; | ||
| }; | ||
|
|
||
| auto traverse = [this, &timeout_reached, &framesToRoot]( |
There was a problem hiding this comment.
invalid case style for variable framesToRoot
| auto traverse = [this, &timeout_reached, &framesToRoot]( | |
| auto traverse = [this, &timeout_reached, &frames_to_root]( |
| !framesToRoot(t1, last_tf_tree, frames_from_t1) || | ||
| !framesToRoot(t2, last_tf_tree, frames_from_t2)) { |
There was a problem hiding this comment.
invalid case style for variable framesToRoot
| !framesToRoot(t1, last_tf_tree, frames_from_t1) || | |
| !framesToRoot(t2, last_tf_tree, frames_from_t2)) { | |
| !frames_to_root(t1, last_tf_tree, frames_from_t1) || | |
| !frames_to_root(t2, last_tf_tree, frames_from_t2)) { |
| explicit ManagedTransformBuffer(rclcpp::Node * node, bool managed = true); | ||
| explicit ManagedTransformBuffer( | ||
| rclcpp::Clock::SharedPtr clock, | ||
| tf2::Duration cache_time = tf2::Duration(tf2::BUFFER_CORE_DEFAULT_CACHE_TIME)); | ||
|
|
||
| /** @brief Destroy the Managed Transform Buffer object */ | ||
| ~ManagedTransformBuffer(); |
There was a problem hiding this comment.
class ManagedTransformBuffer can be made trivially destructible by defaulting the destructor on its first declaration
| ~ManagedTransformBuffer(); | |
| ~ManagedTransformBuffer() = default; |
| deactivateListener(); | ||
| deactivateLocalListener(); | ||
| } | ||
| ManagedTransformBuffer::~ManagedTransformBuffer() = default; |
There was a problem hiding this comment.
class ManagedTransformBuffer can be made trivially destructible by defaulting the destructor on its first declaration
| ManagedTransformBuffer::~ManagedTransformBuffer() = default; |
Description
Tests performed
Not applicable.
Effects on system behavior
Not applicable.
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.