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

feat(autoware_pointcloud_preprocessor): redesign concatenate and time sync node #8300

Open
wants to merge 137 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
137 commits
Select commit Hold shift + click to select a range
306d8a2
chore: rebase main
vividf Sep 19, 2024
946365a
chore: solve conflicts
vividf Sep 19, 2024
d4978f7
chore: fix cpp check
vividf Aug 1, 2024
63a870b
chore: add diagnostics readme
vividf Aug 1, 2024
c8cca1f
chore: update figure
vividf Aug 1, 2024
89650a2
chore: upload jitter.png and add old design link
vividf Aug 2, 2024
d423a22
chore: add the link to the tool for analyzing timestamp
vividf Aug 2, 2024
3d21b6c
fix: fix bug that timer didn't cancel
vividf Sep 4, 2024
542ce97
chore: fix logic for logging
vividf Sep 5, 2024
77a3a79
Update sensing/autoware_pointcloud_preprocessor/docs/concatenate-data.md
vividf Sep 18, 2024
52030a7
Update sensing/autoware_pointcloud_preprocessor/src/concatenate_data/…
vividf Sep 18, 2024
3ddf249
Update sensing/autoware_pointcloud_preprocessor/schema/cocatenate_and…
vividf Sep 18, 2024
d679736
Update sensing/autoware_pointcloud_preprocessor/schema/cocatenate_and…
vividf Sep 18, 2024
1f9d24c
Update sensing/autoware_pointcloud_preprocessor/src/concatenate_data/…
vividf Sep 18, 2024
09452e7
Update sensing/autoware_pointcloud_preprocessor/src/concatenate_data/…
vividf Sep 18, 2024
d900d3f
chore: remove distortion corrector related changes
vividf Sep 19, 2024
bcbe94a
feat: add managed tf buffer
vividf Sep 19, 2024
22b654a
chore: fix filename
vividf Sep 20, 2024
076bfaa
chore: add explanataion for maximum queue size
vividf Sep 20, 2024
0e59f48
chore: add explanation for timeout_sec
vividf Sep 20, 2024
8e79976
chore: fix schema's explanation
vividf Sep 20, 2024
66b4092
chore: fix description for twist and odom
vividf Sep 20, 2024
10d83ee
chore: remove license that are not used
vividf Sep 20, 2024
fbb5fe9
chore: change guard to prama once
vividf Sep 20, 2024
3f0732e
chore: default value change to string
vividf Sep 20, 2024
c19250b
Update sensing/autoware_pointcloud_preprocessor/test/test_concatenate…
vividf Sep 20, 2024
089e108
Merge branch 'feature/redesign_concatenate_and_time_sync_node' of git…
vividf Sep 20, 2024
5ec228c
Update sensing/autoware_pointcloud_preprocessor/test/test_concatenate…
vividf Sep 20, 2024
f4c869e
Update sensing/autoware_pointcloud_preprocessor/test/test_concatenate…
vividf Sep 20, 2024
4cca6c1
Update sensing/autoware_pointcloud_preprocessor/test/test_concatenate…
vividf Sep 20, 2024
53279b5
style(pre-commit): autofix
pre-commit-ci[bot] Sep 20, 2024
853b8fe
chore: clang-tidy style for static constexpr
vividf Sep 20, 2024
d32dfbc
chore: remove unused vector header
vividf Sep 20, 2024
07cb753
chore: fix naming concatenated_cloud_publisher
vividf Sep 20, 2024
490b15c
chore: fix namimg diagnostic_updater_
vividf Sep 20, 2024
8f16896
chore: specify parameter in comment
vividf Sep 20, 2024
1d89cdd
chore: change RCLCPP_WARN to RCLCPP_WARN_STREAM_THROTTLE
vividf Sep 20, 2024
651c666
chore: add comment for cancelling timer
vividf Sep 20, 2024
0aae2ec
chore: merge remote branch
vividf Sep 20, 2024
370483c
chore: Simplify loop structure for topic-to-cloud mapping
vividf Sep 23, 2024
4c0b585
chore: fix spell errors
vividf Sep 23, 2024
2decb65
chore: fix more spell error
vividf Sep 23, 2024
eeb310d
chore: rename mutex and lock
vividf Sep 25, 2024
9a85a52
chore: const reference for string parameter
vividf Sep 25, 2024
1114898
chore: add explaination for RclcppTimeHash_
vividf Sep 25, 2024
84b547c
chore: change the concatenate node to parent node
vividf Sep 25, 2024
e49d521
chore: clean processOdometry and processTwist
vividf Sep 25, 2024
5ae681c
chore: change twist shared pointer queue to twist queue
vividf Sep 26, 2024
3a8ff07
chore: refactor compensate pointcloud to function
vividf Sep 26, 2024
49b54d4
chore: reallocate memory for concatenate_cloud_ptr
vividf Sep 26, 2024
de94fa6
chore: remove new to make shared
vividf Sep 26, 2024
7979153
chore: dis to distance
vividf Sep 26, 2024
b344427
chore: refacotr poitncloud_sub
vividf Sep 26, 2024
b0c8a7c
chore: return early return but throw runtime error
vividf Sep 26, 2024
fa0c4dc
chore: replace #define DEFAULT_SYNC_TOPIC_POSTFIX with member variable
vividf Sep 26, 2024
66a62d1
chore: fix spell error
vividf Sep 26, 2024
67bfa26
chore: remove redundant function call
vividf Sep 26, 2024
bafaea1
chore: replace conplex tuple to structure
vividf Sep 26, 2024
c1a4001
chore: use reference instead of a pointer to conveys node
vividf Sep 26, 2024
7ebd332
chore: fix camel to snake case
vividf Sep 26, 2024
52ed5ed
chore: fix logic of publish synchronized pointcloud
vividf Sep 27, 2024
b863d49
chore: fix cpp check
vividf Sep 27, 2024
92d69a4
chore: remove logging and throw error directly
vividf Sep 30, 2024
edb0610
chore: fix clangd warnings
vividf Sep 30, 2024
b6700a9
chore: fix json schema
vividf Sep 30, 2024
130bcb8
chore: fix clangd warning
vividf Sep 30, 2024
31500f8
chore: remove unused variable
vividf Sep 30, 2024
000c890
chore: fix launcher
vividf Oct 1, 2024
55e0d24
chore: fix clangd warning
vividf Oct 1, 2024
0611bb9
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Oct 4, 2024
9199a3d
chore: ensure thread safety
vividf Oct 4, 2024
d6c7a48
style(pre-commit): autofix
pre-commit-ci[bot] Oct 4, 2024
4815917
chore: clean code
vividf Oct 7, 2024
40fe11e
chore: add parameters for handling rosbag replay in loops
vividf Oct 7, 2024
3433bf0
chore: fix diagonistic
vividf Oct 7, 2024
09b8ce3
chore: reduce copy operation
vividf Oct 21, 2024
782228f
chore: reserve space for concatenated pointcloud
vividf Oct 21, 2024
3606114
chore: fix clangd error
vividf Oct 21, 2024
6ed7537
chore: fix pipeline latency
vividf Oct 21, 2024
0248a24
chore: add debug mode
vividf Oct 21, 2024
76d3b4c
chore: refactor convert_to_xyzirc_cloud function
vividf Oct 22, 2024
e709d37
chore: fix json schema
vividf Oct 22, 2024
fcdb989
chore: fix logging output
vividf Oct 22, 2024
460b467
chore: fix the output order of the debug mode
vividf Oct 22, 2024
798cbd6
chore: fix pipeline latency output
vividf Oct 23, 2024
a2e8b77
chore: clean code
vividf Oct 24, 2024
2562d6e
chore: set some parameters to false in testing
vividf Oct 24, 2024
a970f79
chore: fix default value for schema
vividf Oct 24, 2024
4d95a01
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Oct 25, 2024
4e39bbc
chore: fix diagnostic msgs
vividf Oct 28, 2024
fe2e5c2
Merge branch 'feature/redesign_concatenate_and_time_sync_node' of git…
vividf Oct 28, 2024
50036b5
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Oct 29, 2024
afa000d
chore: fix parameter for sample ros bag
vividf Oct 29, 2024
19170a1
chore: autoware point type namespace
vividf Nov 7, 2024
e4dea9f
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Nov 7, 2024
d0d5b51
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Nov 11, 2024
3123849
chore: update readme
vividf Nov 13, 2024
ce2119b
Merge branch 'feature/redesign_concatenate_and_time_sync_node' of git…
vividf Nov 13, 2024
e115071
chore: fix empty pointcloud
vividf Nov 22, 2024
1b2f2b2
Merge branch 'autowarefoundation:main' into feature/redesign_concaten…
vividf Nov 22, 2024
5bcb02d
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Dec 2, 2024
7224ad5
Merge branch 'feature/redesign_concatenate_and_time_sync_node' of git…
vividf Dec 2, 2024
16d1bce
chore: remove duplicated logic
vividf Dec 2, 2024
8d7d1e8
chore: fix logic for handling empty pointcloud
vividf Dec 2, 2024
ba2a3af
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Dec 2, 2024
7b20619
chore: clean code
vividf Dec 2, 2024
972758f
chore: remove rosbag_replay parameter
vividf Dec 2, 2024
e6c6f6a
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Dec 4, 2024
b950f4a
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Dec 6, 2024
dde8466
chore: remove nodelet cpp
vividf Dec 6, 2024
1cc54ac
chore: clang tidy warning
vividf Dec 6, 2024
46fd14d
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Dec 6, 2024
aa89a1f
feat: add naive approach for unsynchronized pointclouds
vividf Dec 9, 2024
76ac99f
chore: add more explanations in docs for naive approach
vividf Dec 9, 2024
edea4e7
Merge branch 'feature/redesign_concatenate_and_time_sync_node' of git…
vividf Dec 9, 2024
fb9bebc
feat: refactor naive method and fix the multithreading issue
vividf Dec 11, 2024
f944277
chore: set parameter to naive
vividf Dec 12, 2024
5a10484
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Dec 13, 2024
b5cd104
chore: fix parameter
vividf Dec 13, 2024
3a24c26
Merge branch 'feature/redesign_concatenate_and_time_sync_node' of git…
vividf Dec 13, 2024
ab65b2f
chore: fix readme
vividf Dec 20, 2024
a69838f
Update sensing/autoware_pointcloud_preprocessor/docs/concatenate-data.md
vividf Dec 20, 2024
0487272
Update sensing/autoware_pointcloud_preprocessor/docs/concatenate-data.md
vividf Dec 20, 2024
3e9d8f4
style(pre-commit): autofix
pre-commit-ci[bot] Dec 20, 2024
71a1441
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Dec 20, 2024
f13a4b5
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Dec 23, 2024
1dd7fbc
feat: remove mutually exclusive approaches
vividf Dec 23, 2024
9c1f870
chore: fix spell error
vividf Dec 23, 2024
2efabf4
chore: remove unused variable
vividf Dec 24, 2024
fd7db92
refactor: refactor collectorInfo to polymorphic
vividf Dec 24, 2024
f278aea
chore: fix variable name
vividf Dec 24, 2024
9523a3b
chore: fix figure and diagnostic msg in readme
vividf Dec 26, 2024
d01eba8
chroe: refactor collectorinfo structure
vividf Jan 6, 2025
f8fc63c
chore: revert wrong file changes
vividf Jan 6, 2025
609290d
Merge branch 'main' into feature/redesign_concatenate_and_time_sync_node
vividf Jan 9, 2025
074ab67
chore: improve message
vividf Jan 10, 2025
ccd5aab
chore: remove unused input topics
vividf Jan 10, 2025
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
46 changes: 22 additions & 24 deletions sensing/autoware_pointcloud_preprocessor/docs/concatenate-data.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ For example, consider a vehicle equipped with three LiDAR sensors mounted on the

After processing the data through the `concatenate_and_time_synchronize_node`, the outputs from all LiDARs are combined into a single comprehensive point cloud that provides a complete view of the environment:

<div style="margin: 0 auto; max-width: 500px" markdown="1">
![Full Scene View](./image/concatenate_all.png)
</div>
![Full Scene View](./image/concatenate_all.png)

This resulting point cloud allows autonomous systems to detect obstacles, map the environment, and navigate more effectively, leveraging the complementary fields of view from multiple LiDAR sensors.

Expand Down Expand Up @@ -130,16 +128,16 @@ colcon test --packages-select autoware_pointcloud_preprocessor --event-handlers

## Debug and Diagnostics

To verify whether the node has successfully concatenated the point clouds, the user can examine the `/diagnostics` topic using the following command:
To verify whether the node has successfully concatenated the point clouds, the user can examine rqt or the `/diagnostics` topic using the following command:

```bash
ros2 topic echo /diagnostics
```

Below is an example output when the point clouds are concatenated successfully:

- Each point cloud has a value of `1`.
- The `concatenate status` is `1`.
- Each point cloud has a value of `True`.
- The `cloud concatenation success` is `True`.
- The `level` value is `\0`. (diagnostic_msgs::msg::DiagnosticStatus::OK)

```bash
Expand All @@ -162,24 +160,24 @@ status:
value: '1718260240.169229984'
- key: /sensing/lidar/left/pointcloud_before_sync timestamp
value: '1718260240.159229994'
- key: /sensing/lidar/left/pointcloud_before_sync
value: '1'
- key: /sensing/lidar/left/pointcloud_before_sync is concatenated
value: 'True'
- key: /sensing/lidar/right/pointcloud_before_sync timestamp
value: '1718260240.194104910'
- key: /sensing/lidar/right/pointcloud_before_sync
value: '1'
- key: /sensing/lidar/right/pointcloud_before_sync is concatenated
value: 'True'
- key: /sensing/lidar/top/pointcloud_before_sync timestamp
value: '1718260240.234578133'
- key: /sensing/lidar/top/pointcloud_before_sync
value: '1'
- key: concatenate status
value: '1'
- key: /sensing/lidar/top/pointcloud_before_sync is concatenated
value: 'True'
- key: cloud concatenation success
value: 'True'
```

Below is an example when point clouds fail to concatenate successfully.

- Some point clouds might have values of `0`.
- The `concatenate status` is `0`.
- Some point clouds might have values of `False`.
- The `cloud concatenation success` is `False`.
- The `level` value is `\x02`. (diagnostic_msgs::msg::DiagnosticStatus::ERROR)

```bash
Expand All @@ -202,16 +200,16 @@ status:
value: '1718260240.869827986'
- key: /sensing/lidar/left/pointcloud_before_sync timestamp
value: '1718260240.859827995'
- key: /sensing/lidar/left/pointcloud_before_sync
value: '1'
- key: /sensing/lidar/left/pointcloud_before_sync is concatenated
value: 'True'
- key: /sensing/lidar/right/pointcloud_before_sync timestamp
value: '1718260240.895193815'
- key: /sensing/lidar/right/pointcloud_before_sync
value: '1'
- key: /sensing/lidar/top/pointcloud_before_sync
value: '0'
- key: concatenate status
value: '0'
- key: /sensing/lidar/right/pointcloud_before_sync is concatenated
value: 'True'
- key: /sensing/lidar/top/pointcloud_before_sync is concatenated
value: 'False'
- key: cloud concatenation success
value: 'False'
```

## Node separation options
Expand Down
vividf marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,31 @@

enum class CollectorStrategyType { Naive, Advanced };

struct CollectorInfo
struct CollectorInfoBase
{
virtual ~CollectorInfoBase() = default;
[[nodiscard]] virtual CollectorStrategyType getStrategyType() const = 0;
};

struct NaiveCollectorInfo : public CollectorInfoBase
{
double timestamp{0.0};

[[nodiscard]] CollectorStrategyType getStrategyType() const override

Check warning on line 42 in sensing/autoware_pointcloud_preprocessor/include/autoware/pointcloud_preprocessor/concatenate_data/cloud_collector.hpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/include/autoware/pointcloud_preprocessor/concatenate_data/cloud_collector.hpp#L42

Added line #L42 was not covered by tests
{
return CollectorStrategyType::Naive;

Check warning on line 44 in sensing/autoware_pointcloud_preprocessor/include/autoware/pointcloud_preprocessor/concatenate_data/cloud_collector.hpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/include/autoware/pointcloud_preprocessor/concatenate_data/cloud_collector.hpp#L44

Added line #L44 was not covered by tests
}
};

struct AdvancedCollectorInfo : public CollectorInfoBase
{
double timestamp{0.0};
double noise_window{0.0};
CollectorStrategyType strategy_type{CollectorStrategyType::Naive};

[[nodiscard]] CollectorStrategyType getStrategyType() const override

Check warning on line 53 in sensing/autoware_pointcloud_preprocessor/include/autoware/pointcloud_preprocessor/concatenate_data/cloud_collector.hpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/include/autoware/pointcloud_preprocessor/concatenate_data/cloud_collector.hpp#L53

Added line #L53 was not covered by tests
{
return CollectorStrategyType::Advanced;

Check warning on line 55 in sensing/autoware_pointcloud_preprocessor/include/autoware/pointcloud_preprocessor/concatenate_data/cloud_collector.hpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/include/autoware/pointcloud_preprocessor/concatenate_data/cloud_collector.hpp#L55

Added line #L55 was not covered by tests
}
};

class CloudCollector
Expand All @@ -54,10 +74,10 @@
std::unordered_map<std::string, sensor_msgs::msg::PointCloud2::SharedPtr>
get_topic_to_cloud_map();

bool concatenate_finished() const;
[[nodiscard]] bool concatenate_finished() const;

void set_info(CollectorInfo collector_info);
CollectorInfo get_info() const;
void set_info(std::shared_ptr<CollectorInfoBase> collector_info);
[[nodiscard]] std::shared_ptr<CollectorInfoBase> get_info() const;
void show_debug_message();

private:
Expand All @@ -70,7 +90,7 @@
bool debug_mode_;
bool concatenate_finished_{false};
std::mutex concatenate_mutex_;
CollectorInfo collector_info_;
std::shared_ptr<CollectorInfoBase> collector_info_;
};

} // namespace autoware::pointcloud_preprocessor
vividf marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ class PointCloudConcatenateDataSynchronizerComponent : public rclcpp::Node
explicit PointCloudConcatenateDataSynchronizerComponent(const rclcpp::NodeOptions & node_options);
~PointCloudConcatenateDataSynchronizerComponent() override = default;
void publish_clouds(
ConcatenatedCloudResult && concatenated_cloud_result, CollectorInfo collector_info);
ConcatenatedCloudResult && concatenated_cloud_result,
std::shared_ptr<CollectorInfoBase> collector_info);
void manage_collector_list();
std::list<std::shared_ptr<CloudCollector>> get_cloud_collectors();
void add_cloud_collector(const std::shared_ptr<CloudCollector> & collector);
Expand Down Expand Up @@ -84,7 +85,7 @@ class PointCloudConcatenateDataSynchronizerComponent : public rclcpp::Node
bool drop_previous_but_late_pointcloud_{false};
bool publish_pointcloud_{false};
bool is_concatenated_cloud_empty_{false};
CollectorInfo diagnostic_collector_info_;
std::shared_ptr<CollectorInfoBase> diagnostic_collector_info_;
std::unordered_map<std::string, double> diagnostic_topic_to_original_stamp_map_;

std::shared_ptr<CombineCloudHandler> combine_cloud_handler_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,19 @@
});
}

void CloudCollector::set_info(CollectorInfo collector_info)
void CloudCollector::set_info(std::shared_ptr<CollectorInfoBase> collector_info)
{
collector_info_ = collector_info;
collector_info_ = std::move(collector_info);
}

CollectorInfo CloudCollector::get_info() const
std::shared_ptr<CollectorInfoBase> CloudCollector::get_info() const
{
return collector_info_;
}

bool CloudCollector::topic_exists(const std::string & topic_name)

Check warning on line 61 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp#L61

Added line #L61 was not covered by tests
{
return topic_to_cloud_map_.find(topic_name) != topic_to_cloud_map_.end();

Check warning on line 63 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp#L63

Added line #L63 was not covered by tests
}

bool CloudCollector::process_pointcloud(
Expand All @@ -72,7 +72,7 @@
// Check if the map already contains an entry for the same topic. This shouldn't happen if the
// parameter 'lidar_timestamp_noise_window' is set correctly.
if (topic_to_cloud_map_.find(topic_name) != topic_to_cloud_map_.end()) {
RCLCPP_WARN_STREAM_THROTTLE(

Check warning on line 75 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp#L75

Added line #L75 was not covered by tests
ros2_parent_node_->get_logger(), *ros2_parent_node_->get_clock(),
std::chrono::milliseconds(10000).count(),
"Topic '" << topic_name
Expand All @@ -94,7 +94,7 @@
void CloudCollector::concatenate_callback()
{
if (debug_mode_) {
show_debug_message();

Check warning on line 97 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp#L97

Added line #L97 was not covered by tests
}

// All pointclouds are received or the timer has timed out, cancel the timer and concatenate the
Expand All @@ -120,22 +120,26 @@
return topic_to_cloud_map_;
}

void CloudCollector::show_debug_message()

Check warning on line 123 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp#L123

Added line #L123 was not covered by tests
{
auto time_until_trigger = timer_->time_until_trigger();
std::stringstream log_stream;

Check warning on line 126 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp#L125-L126

Added lines #L125 - L126 were not covered by tests
log_stream << std::fixed << std::setprecision(6);
log_stream << "Collector's concatenate callback time: "
<< ros2_parent_node_->get_clock()->now().seconds() << " seconds\n";

if (collector_info_.strategy_type == CollectorStrategyType::Advanced) {
log_stream << "Advanced strategy:\n Collector's reference time min: "
<< collector_info_.timestamp - collector_info_.noise_window
<< " to max: " << collector_info_.timestamp + collector_info_.noise_window
<< " seconds\n";
} else if (collector_info_.strategy_type == CollectorStrategyType::Naive) {
log_stream << "Naive strategy:\n Collector's first cloud arrival time: "
<< collector_info_.timestamp << " seconds\n";
if (collector_info_) {
if (collector_info_->getStrategyType() == CollectorStrategyType::Advanced) {
auto advanced_info = std::dynamic_pointer_cast<AdvancedCollectorInfo>(collector_info_);

Check warning on line 133 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp#L133

Added line #L133 was not covered by tests
log_stream << "Advanced strategy:\n Collector's reference time min: "
<< advanced_info->timestamp - advanced_info->noise_window
<< " to max: " << advanced_info->timestamp + advanced_info->noise_window
<< " seconds\n";
} else if (collector_info_->getStrategyType() == CollectorStrategyType::Naive) {
auto naive_info = std::dynamic_pointer_cast<NaiveCollectorInfo>(collector_info_);

Check warning on line 139 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp#L139

Added line #L139 was not covered by tests
log_stream << "Naive strategy:\n Collector's timestamp: " << naive_info->timestamp
<< " seconds\n";
}
mojomex marked this conversation as resolved.
Show resolved Hide resolved
}

log_stream << "Time until trigger: " << (time_until_trigger.count() / 1e9) << " seconds\n";
Expand All @@ -151,6 +155,6 @@
log_stream << "]\n";

RCLCPP_INFO(ros2_parent_node_->get_logger(), "%s", log_stream.str().c_str());
}

Check warning on line 158 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/cloud_collector.cpp#L158

Added line #L158 was not covered by tests

} // namespace autoware::pointcloud_preprocessor
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@
namespace autoware::pointcloud_preprocessor
{

NaiveMatchingStrategy::NaiveMatchingStrategy(rclcpp::Node & node)

Check warning on line 29 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp#L29

Added line #L29 was not covered by tests
{
strategy_type_ = CollectorStrategyType::Naive;

Check warning on line 31 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp#L31

Added line #L31 was not covered by tests
RCLCPP_INFO(node.get_logger(), "Utilize naive matching strategy");
}

Check warning on line 33 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp#L33

Added line #L33 was not covered by tests

std::optional<std::shared_ptr<CloudCollector>> NaiveMatchingStrategy::match_cloud_to_collector(

Check warning on line 35 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp#L35

Added line #L35 was not covered by tests
const std::list<std::shared_ptr<CloudCollector>> & cloud_collectors,
const MatchingParams & params) const
{
Expand All @@ -41,27 +41,27 @@

for (const auto & cloud_collector : cloud_collectors) {
if (!cloud_collector->topic_exists(params.topic_name)) {
CollectorInfo collector_info = cloud_collector->get_info();
double time_difference = std::abs(params.cloud_arrival_time - collector_info.timestamp);
if (!smallest_time_difference || time_difference < smallest_time_difference) {
smallest_time_difference = time_difference;
closest_collector = cloud_collector;
auto info = cloud_collector->get_info();
if (auto naive_info = std::dynamic_pointer_cast<NaiveCollectorInfo>(info)) {
double time_difference = std::abs(params.cloud_arrival_time - naive_info->timestamp);
if (!smallest_time_difference || time_difference < smallest_time_difference) {
smallest_time_difference = time_difference;
closest_collector = cloud_collector;
}
}
}
}

return closest_collector;

Check warning on line 55 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp#L55

Added line #L55 was not covered by tests
}

Check warning on line 56 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Deep, Nested Complexity

NaiveMatchingStrategy::match_cloud_to_collector has a nested complexity depth of 4, threshold = 4. This function contains deeply nested logic such as if statements and/or loops. The deeper the nesting, the lower the code health.

void NaiveMatchingStrategy::set_collector_info(

Check warning on line 58 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp#L58

Added line #L58 was not covered by tests
std::shared_ptr<CloudCollector> & collector, const MatchingParams & matching_params)
{
CollectorInfo collector_info;
;
collector_info.timestamp = matching_params.cloud_arrival_time;
collector_info.strategy_type = strategy_type_;
collector->set_info(collector_info);
auto info = std::make_shared<NaiveCollectorInfo>();
info->timestamp = matching_params.cloud_arrival_time;

Check warning on line 62 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp#L62

Added line #L62 was not covered by tests
collector->set_info(info);
}

Check warning on line 64 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp#L64

Added line #L64 was not covered by tests

AdvancedMatchingStrategy::AdvancedMatchingStrategy(
rclcpp::Node & node, std::vector<std::string> input_topics)
Expand All @@ -74,11 +74,11 @@

if (lidar_timestamp_offsets.size() != input_topics.size()) {
throw std::runtime_error(
"The number of topics does not match the number of timestamp offsets.");

Check warning on line 77 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp#L77

Added line #L77 was not covered by tests
}
if (lidar_timestamp_noise_window.size() != input_topics.size()) {
throw std::runtime_error(
"The number of topics does not match the number of timestamp noise window.");

Check warning on line 81 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/collector_matching_strategy.cpp#L81

Added line #L81 was not covered by tests
}

for (size_t i = 0; i < input_topics.size(); i++) {
Expand All @@ -96,14 +96,16 @@
const MatchingParams & params) const
{
for (const auto & cloud_collector : cloud_collectors) {
CollectorInfo collector_info = cloud_collector->get_info();
auto reference_timestamp_min = collector_info.timestamp - collector_info.noise_window;
auto reference_timestamp_max = collector_info.timestamp + collector_info.noise_window;
double time = params.cloud_timestamp - topic_to_offset_map_.at(params.topic_name);
if (
time < reference_timestamp_max + topic_to_noise_window_map_.at(params.topic_name) &&
time > reference_timestamp_min - topic_to_noise_window_map_.at(params.topic_name)) {
return cloud_collector;
auto info = cloud_collector->get_info();
if (auto advanced_info = std::dynamic_pointer_cast<AdvancedCollectorInfo>(info)) {
auto reference_timestamp_min = advanced_info->timestamp - advanced_info->noise_window;
auto reference_timestamp_max = advanced_info->timestamp + advanced_info->noise_window;
double time = params.cloud_timestamp - topic_to_offset_map_.at(params.topic_name);
if (
time < reference_timestamp_max + topic_to_noise_window_map_.at(params.topic_name) &&
time > reference_timestamp_min - topic_to_noise_window_map_.at(params.topic_name)) {
return cloud_collector;
}
}
}
return std::nullopt;
Expand All @@ -112,13 +114,11 @@
void AdvancedMatchingStrategy::set_collector_info(
std::shared_ptr<CloudCollector> & collector, const MatchingParams & matching_params)
{
CollectorInfo collector_info;
;
collector_info.timestamp =
auto info = std::make_shared<AdvancedCollectorInfo>();
info->timestamp =
matching_params.cloud_timestamp - topic_to_offset_map_[matching_params.topic_name];
collector_info.noise_window = topic_to_noise_window_map_[matching_params.topic_name];
collector_info.strategy_type = strategy_type_;
collector->set_info(collector_info);
info->noise_window = topic_to_noise_window_map_[matching_params.topic_name];
collector->set_info(info);
}

} // namespace autoware::pointcloud_preprocessor
vividf marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2024 TIER IV, Inc.

Check warning on line 1 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Overall Code Complexity

This module has a mean cyclomatic complexity of 5.73 across 11 functions. The mean complexity threshold is 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -33,85 +33,85 @@
namespace autoware::pointcloud_preprocessor
{

PointCloudConcatenateDataSynchronizerComponent::PointCloudConcatenateDataSynchronizerComponent(
const rclcpp::NodeOptions & node_options)
: Node("point_cloud_concatenator_component", node_options)
{
// initialize debug tool
using autoware::universe_utils::DebugPublisher;
using autoware::universe_utils::StopWatch;
stop_watch_ptr_ = std::make_unique<StopWatch<std::chrono::milliseconds>>();
debug_publisher_ = std::make_unique<DebugPublisher>(this, "concatenate_data_synchronizer");
stop_watch_ptr_->tic("cyclic_time");
stop_watch_ptr_->tic("processing_time");

// initialize parameters
params_.debug_mode = declare_parameter<bool>("debug_mode");
params_.has_static_tf_only = declare_parameter<bool>("has_static_tf_only");
params_.rosbag_length = declare_parameter<double>("rosbag_length");
params_.maximum_queue_size = static_cast<size_t>(declare_parameter<int>("maximum_queue_size"));
params_.timeout_sec = declare_parameter<double>("timeout_sec");
params_.is_motion_compensated = declare_parameter<bool>("is_motion_compensated");
params_.publish_synchronized_pointcloud =
declare_parameter<bool>("publish_synchronized_pointcloud");
params_.keep_input_frame_in_synchronized_pointcloud =
declare_parameter<bool>("keep_input_frame_in_synchronized_pointcloud");
params_.publish_previous_but_late_pointcloud =
declare_parameter<bool>("publish_previous_but_late_pointcloud");
params_.synchronized_pointcloud_postfix =
declare_parameter<std::string>("synchronized_pointcloud_postfix");
params_.input_twist_topic_type = declare_parameter<std::string>("input_twist_topic_type");
params_.input_topics = declare_parameter<std::vector<std::string>>("input_topics");
params_.output_frame = declare_parameter<std::string>("output_frame");

if (params_.input_topics.empty()) {
throw std::runtime_error("Need a 'input_topics' parameter to be set before continuing.");
}
if (params_.input_topics.size() == 1) {
throw std::runtime_error("Only one topic given. Need at least two topics to continue.");
}

if (params_.output_frame.empty()) {
throw std::runtime_error("Need an 'output_frame' parameter to be set before continuing.");
}

params_.matching_strategy = declare_parameter<std::string>("matching_strategy.type");
if (params_.matching_strategy == "naive") {
collector_matching_strategy_ = std::make_unique<NaiveMatchingStrategy>(*this);
} else if (params_.matching_strategy == "advanced") {
collector_matching_strategy_ =
std::make_unique<AdvancedMatchingStrategy>(*this, params_.input_topics);
} else {
throw std::runtime_error("Matching strategy must be 'advanced' or 'naive'");
}

// Publishers
concatenated_cloud_publisher_ = this->create_publisher<sensor_msgs::msg::PointCloud2>(
"output", rclcpp::SensorDataQoS().keep_last(params_.maximum_queue_size));

// Transformed Raw PointCloud2 Publisher to publish the transformed pointcloud
if (params_.publish_synchronized_pointcloud) {
for (auto & topic : params_.input_topics) {
std::string new_topic =
replace_sync_topic_name_postfix(topic, params_.synchronized_pointcloud_postfix);
auto publisher = this->create_publisher<sensor_msgs::msg::PointCloud2>(
new_topic, rclcpp::SensorDataQoS().keep_last(params_.maximum_queue_size));
topic_to_transformed_cloud_publisher_map_.insert({topic, publisher});
}
}

// Subscribers
if (params_.is_motion_compensated) {
if (params_.input_twist_topic_type == "twist") {
twist_sub_ = this->create_subscription<geometry_msgs::msg::TwistWithCovarianceStamped>(
"~/input/twist", rclcpp::QoS{100},
std::bind(
&PointCloudConcatenateDataSynchronizerComponent::twist_callback, this,
std::placeholders::_1));
} else if (params_.input_twist_topic_type == "odom") {
odom_sub_ = this->create_subscription<nav_msgs::msg::Odometry>(
"~/input/odom", rclcpp::QoS{100},
std::bind(

Check warning on line 114 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp#L114

Added line #L114 was not covered by tests
&PointCloudConcatenateDataSynchronizerComponent::odom_callback, this,
std::placeholders::_1));
} else {
Expand Down Expand Up @@ -178,24 +178,24 @@
return replaced_topic_name;
}

void PointCloudConcatenateDataSynchronizerComponent::cloud_callback(
const sensor_msgs::msg::PointCloud2::SharedPtr & input_ptr, const std::string & topic_name)
{
stop_watch_ptr_->toc("processing_time", true);
double cloud_arrival_time = this->get_clock()->now().seconds();
manage_collector_list();

if (!utils::is_data_layout_compatible_with_point_xyzirc(*input_ptr)) {
RCLCPP_ERROR(
get_logger(), "The pointcloud layout is not compatible with PointXYZIRC. Aborting");

if (utils::is_data_layout_compatible_with_point_xyzi(*input_ptr)) {
RCLCPP_ERROR(
get_logger(),
"The pointcloud layout is compatible with PointXYZI. You may be using legacy code/data");
}

return;

Check warning on line 198 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp#L198

Added line #L198 was not covered by tests
}

if (params_.debug_mode) {
Expand Down Expand Up @@ -253,19 +253,20 @@
combine_cloud_handler_->process_twist(input);
}

void PointCloudConcatenateDataSynchronizerComponent::odom_callback(

Check warning on line 256 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp#L256

Added line #L256 was not covered by tests
const nav_msgs::msg::Odometry::ConstSharedPtr input)
{
combine_cloud_handler_->process_odometry(input);
}

Check warning on line 260 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp#L259-L260

Added lines #L259 - L260 were not covered by tests

void PointCloudConcatenateDataSynchronizerComponent::publish_clouds(
ConcatenatedCloudResult && concatenated_cloud_result, CollectorInfo collector_info)
ConcatenatedCloudResult && concatenated_cloud_result,
std::shared_ptr<CollectorInfoBase> collector_info)
{
// should never come to this state.
if (concatenated_cloud_result.concatenate_cloud_ptr == nullptr) {
RCLCPP_ERROR(this->get_logger(), "Concatenated cloud is a nullptr.");
return;

Check warning on line 269 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp#L269

Added line #L269 was not covered by tests
}

if (concatenated_cloud_result.concatenate_cloud_ptr->data.empty()) {
Expand All @@ -280,12 +281,12 @@
current_concatenate_cloud_timestamp_ < latest_concatenate_cloud_timestamp_ &&
!params_.publish_previous_but_late_pointcloud) {
// Publish the cloud if the rosbag replays in loop
if (
latest_concatenate_cloud_timestamp_ - current_concatenate_cloud_timestamp_ >

Check warning on line 285 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp#L284-L285

Added lines #L284 - L285 were not covered by tests
params_.rosbag_length) {
publish_pointcloud_ = true; // Force publishing in this case

Check warning on line 287 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp#L287

Added line #L287 was not covered by tests
} else {
drop_previous_but_late_pointcloud_ = true; // Otherwise, drop the late pointcloud

Check warning on line 289 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp

View check run for this annotation

Codecov / codecov/patch

sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp#L289

Added line #L289 was not covered by tests
}
} else {
// Publish pointcloud if timestamps are valid or the condition doesn't apply
Expand Down Expand Up @@ -362,82 +363,82 @@
return oss.str();
}

void PointCloudConcatenateDataSynchronizerComponent::check_concat_status(
diagnostic_updater::DiagnosticStatusWrapper & stat)
{
if (publish_pointcloud_ || drop_previous_but_late_pointcloud_) {
stat.add(
"concatenated cloud timestamp", format_timestamp(current_concatenate_cloud_timestamp_));

if (diagnostic_collector_info_.strategy_type == CollectorStrategyType::Naive) {
stat.add(
"first cloud's arrival timestamp", format_timestamp(diagnostic_collector_info_.timestamp));
} else if (diagnostic_collector_info_.strategy_type == CollectorStrategyType::Advanced) {
if (
auto naive_info = std::dynamic_pointer_cast<NaiveCollectorInfo>(diagnostic_collector_info_)) {
stat.add("first cloud's arrival timestamp", format_timestamp(naive_info->timestamp));
} else if (
auto advanced_info =
std::dynamic_pointer_cast<AdvancedCollectorInfo>(diagnostic_collector_info_)) {
stat.add(
"reference timestamp min",
format_timestamp(
diagnostic_collector_info_.timestamp - diagnostic_collector_info_.noise_window));
format_timestamp(advanced_info->timestamp - advanced_info->noise_window));
stat.add(
"reference timestamp max",
format_timestamp(
diagnostic_collector_info_.timestamp + diagnostic_collector_info_.noise_window));
format_timestamp(advanced_info->timestamp + advanced_info->noise_window));
}

bool topic_miss = false;

int concatenated_cloud_status = 1; // Status of concatenated cloud, 1: success, 0: failure
bool concatenation_success = true;
for (const auto & topic : params_.input_topics) {
int cloud_status = 1; // Status of each lidar's pointcloud
bool input_cloud_concatenated = true;
if (
diagnostic_topic_to_original_stamp_map_.find(topic) !=
diagnostic_topic_to_original_stamp_map_.end()) {
stat.add(
topic + " timestamp", format_timestamp(diagnostic_topic_to_original_stamp_map_[topic]));
} else {
topic_miss = true;
concatenated_cloud_status = 0;
cloud_status = 0;
concatenation_success = false;
input_cloud_concatenated = false;
}
stat.add(topic, cloud_status);
stat.add(topic + " is concatenated", input_cloud_concatenated);
}

stat.add("concatenate status", concatenated_cloud_status);
stat.add("cloud concatenation success", concatenation_success);

int8_t level = diagnostic_msgs::msg::DiagnosticStatus::OK;
std::string message = "Concatenated pointcloud is published and include all topics";

if (drop_previous_but_late_pointcloud_) {
if (topic_miss) {
level = diagnostic_msgs::msg::DiagnosticStatus::ERROR;
message =
"Concatenated pointcloud misses some topics and is not published because it arrived "
"too late";
} else {
level = diagnostic_msgs::msg::DiagnosticStatus::ERROR;
message = "Concatenated pointcloud is not published as it is too late";
}
} else {
if (is_concatenated_cloud_empty_) {
level = diagnostic_msgs::msg::DiagnosticStatus::ERROR;
message = "Concatenated pointcloud is empty";
} else if (topic_miss) {
level = diagnostic_msgs::msg::DiagnosticStatus::ERROR;
message = "Concatenated pointcloud is published but misses some topics";
}
}

stat.summary(level, message);

publish_pointcloud_ = false;
drop_previous_but_late_pointcloud_ = false;
is_concatenated_cloud_empty_ = false;
} else {
const int8_t level = diagnostic_msgs::msg::DiagnosticStatus::OK;
const std::string message =
"Concatenate node launch successfully, but waiting for input pointcloud";
stat.summary(level, message);
}
}

Check warning on line 441 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Complex Method

PointCloudConcatenateDataSynchronizerComponent::check_concat_status has a cyclomatic complexity of 11, 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 441 in sensing/autoware_pointcloud_preprocessor/src/concatenate_data/concatenate_and_time_sync_node.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Bumpy Road Ahead

PointCloudConcatenateDataSynchronizerComponent::check_concat_status 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.

std::list<std::shared_ptr<CloudCollector>>
PointCloudConcatenateDataSynchronizerComponent::get_cloud_collectors()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2024 TIER IV, Inc.

Check warning on line 1 in sensing/autoware_pointcloud_preprocessor/test/test_concatenate_node_unit.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Primitive Obsession

In this module, 93.3% of all function arguments are primitive types, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -89,24 +89,24 @@
}
}

static geometry_msgs::msg::TransformStamped generate_transform_msg(
const std::string & parent_frame, const std::string & child_frame, double x, double y, double z,
double qx, double qy, double qz, double qw)
{
rclcpp::Time timestamp(timestamp_seconds, timestamp_nanoseconds, RCL_ROS_TIME);
geometry_msgs::msg::TransformStamped tf_msg;
tf_msg.header.stamp = timestamp;
tf_msg.header.frame_id = parent_frame;
tf_msg.child_frame_id = child_frame;
tf_msg.transform.translation.x = x;
tf_msg.transform.translation.y = y;
tf_msg.transform.translation.z = z;
tf_msg.transform.rotation.x = qx;
tf_msg.transform.rotation.y = qy;
tf_msg.transform.rotation.z = qz;
tf_msg.transform.rotation.w = qw;
return tf_msg;
}

Check warning on line 109 in sensing/autoware_pointcloud_preprocessor/test/test_concatenate_node_unit.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Excess Number of Function Arguments

generate_transform_msg has 9 arguments, threshold = 4. This function has too many arguments, indicating a lack of encapsulation. Avoid adding more arguments.

static sensor_msgs::msg::PointCloud2 generate_pointcloud_msg(
bool generate_points, bool is_lidar_frame, const std::string & topic_name,
Expand Down Expand Up @@ -187,19 +187,19 @@
};

//////////////////////////////// Test combine_cloud_handler ////////////////////////////////
TEST_F(ConcatenateCloudTest, TestProcessTwist)
{
auto twist_msg = std::make_shared<geometry_msgs::msg::TwistWithCovarianceStamped>();
twist_msg->header.stamp = rclcpp::Time(10, 0);
twist_msg->twist.twist.linear.x = 1.0;
twist_msg->twist.twist.angular.z = 0.1;

combine_cloud_handler_->process_twist(twist_msg);

ASSERT_FALSE(combine_cloud_handler_->get_twist_queue().empty());
EXPECT_EQ(combine_cloud_handler_->get_twist_queue().front().twist.linear.x, 1.0);
EXPECT_EQ(combine_cloud_handler_->get_twist_queue().front().twist.angular.z, 0.1);
}

Check warning on line 202 in sensing/autoware_pointcloud_preprocessor/test/test_concatenate_node_unit.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Code Duplication

The module contains 2 functions with similar structure: TEST:ConcatenateCloudTest:TestProcessOdometry,TEST:ConcatenateCloudTest:TestProcessTwist. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

TEST_F(ConcatenateCloudTest, TestProcessOdometry)
{
Expand Down Expand Up @@ -291,165 +291,187 @@

//////////////////////////////// Test cloud_collector ////////////////////////////////

TEST_F(ConcatenateCloudTest, TestSetAndGetCollectorInfo)
TEST_F(ConcatenateCloudTest, TestSetAndGetNaiveCollectorInfo)
{
autoware::pointcloud_preprocessor::CollectorInfo collector_info;
collector_info.timestamp = 10.0;
collector_info.noise_window = 0.1;
collector_->set_info(collector_info);
auto naive_info = std::make_shared<autoware::pointcloud_preprocessor::NaiveCollectorInfo>();
naive_info->timestamp = 15.0;

collector_->set_info(naive_info);
auto collector_info_new = collector_->get_info();

auto naive_info_new =
std::dynamic_pointer_cast<autoware::pointcloud_preprocessor::NaiveCollectorInfo>(
collector_info_new);
ASSERT_NE(naive_info_new, nullptr) << "Collector info is not of type NaiveCollectorInfo";

EXPECT_DOUBLE_EQ(naive_info_new->timestamp, 15.0);
}

TEST_F(ConcatenateCloudTest, TestSetAndGetAdvancedCollectorInfo)
{
auto advanced_info = std::make_shared<autoware::pointcloud_preprocessor::AdvancedCollectorInfo>();
advanced_info->timestamp = 10.0;
advanced_info->noise_window = 0.1;
collector_->set_info(advanced_info);
auto collector_info_new = collector_->get_info();
auto min = collector_info_new.timestamp - collector_info_new.noise_window;
auto max = collector_info_new.timestamp + collector_info_new.noise_window;
auto advanced_info_new =
std::dynamic_pointer_cast<autoware::pointcloud_preprocessor::AdvancedCollectorInfo>(
collector_info_new);
ASSERT_NE(advanced_info_new, nullptr) << "Collector info is not of type AdvancedCollectorInfo";

// Validate the values
auto min = advanced_info_new->timestamp - advanced_info_new->noise_window;
auto max = advanced_info_new->timestamp + advanced_info_new->noise_window;
EXPECT_DOUBLE_EQ(min, 9.9);
EXPECT_DOUBLE_EQ(max, 10.1);
}

TEST_F(ConcatenateCloudTest, TestConcatenateClouds)
{
rclcpp::Time top_timestamp(timestamp_seconds, timestamp_nanoseconds, RCL_ROS_TIME);
rclcpp::Time left_timestamp(timestamp_seconds, timestamp_nanoseconds + 40'000'000, RCL_ROS_TIME);
rclcpp::Time right_timestamp(timestamp_seconds, timestamp_nanoseconds + 80'000'000, RCL_ROS_TIME);
sensor_msgs::msg::PointCloud2 top_pointcloud =
generate_pointcloud_msg(true, false, "lidar_top", top_timestamp);
sensor_msgs::msg::PointCloud2 left_pointcloud =
generate_pointcloud_msg(true, false, "lidar_left", left_timestamp);
sensor_msgs::msg::PointCloud2 right_pointcloud =
generate_pointcloud_msg(true, false, "lidar_right", right_timestamp);

sensor_msgs::msg::PointCloud2::SharedPtr top_pointcloud_ptr =
std::make_shared<sensor_msgs::msg::PointCloud2>(top_pointcloud);
sensor_msgs::msg::PointCloud2::SharedPtr left_pointcloud_ptr =
std::make_shared<sensor_msgs::msg::PointCloud2>(left_pointcloud);
sensor_msgs::msg::PointCloud2::SharedPtr right_pointcloud_ptr =
std::make_shared<sensor_msgs::msg::PointCloud2>(right_pointcloud);

std::unordered_map<std::string, sensor_msgs::msg::PointCloud2::SharedPtr> topic_to_cloud_map;
topic_to_cloud_map["lidar_top"] = top_pointcloud_ptr;
topic_to_cloud_map["lidar_left"] = left_pointcloud_ptr;
topic_to_cloud_map["lidar_right"] = right_pointcloud_ptr;

auto [concatenate_cloud_ptr, topic_to_transformed_cloud_map, topic_to_original_stamp_map] =
collector_->concatenate_pointclouds(topic_to_cloud_map);

// test output concatenate cloud
// No input twist, so it will not do the motion compensation
std::array<Eigen::Vector3f, 10> expected_pointcloud = {
{Eigen::Vector3f(10.0f, 0.0f, 0.0f), Eigen::Vector3f(0.0f, 10.0f, 0.0f),
Eigen::Vector3f(0.0f, 0.0f, 10.0f), Eigen::Vector3f(10.0f, 0.0f, 0.0f),
Eigen::Vector3f(0.0f, 10.0f, 0.0f), Eigen::Vector3f(0.0f, 0.0f, 10.0f),
Eigen::Vector3f(10.0f, 0.0f, 0.0f), Eigen::Vector3f(0.0f, 10.0f, 0.0f),
Eigen::Vector3f(0.0f, 0.0f, 10.0f)}};

size_t i = 0;
std::ostringstream oss;
oss << "Concatenated pointcloud:\n";

sensor_msgs::PointCloud2Iterator<float> iter_x(*concatenate_cloud_ptr, "x");
sensor_msgs::PointCloud2Iterator<float> iter_y(*concatenate_cloud_ptr, "y");
sensor_msgs::PointCloud2Iterator<float> iter_z(*concatenate_cloud_ptr, "z");

for (; iter_x != iter_x.end(); ++iter_x, ++iter_y, ++iter_z, ++i) {
oss << "Concatenated point " << i << ": (" << *iter_x << ", " << *iter_y << ", " << *iter_z
<< ")\n";
EXPECT_FLOAT_EQ(*iter_x, expected_pointcloud[i].x());
EXPECT_FLOAT_EQ(*iter_y, expected_pointcloud[i].y());
EXPECT_FLOAT_EQ(*iter_z, expected_pointcloud[i].z());
}

if (debug_) {
RCLCPP_INFO(concatenate_node_->get_logger(), "%s", oss.str().c_str());
}

// test concatenate cloud has the oldest pointcloud's timestamp
EXPECT_FLOAT_EQ(
top_timestamp.seconds(), rclcpp::Time(concatenate_cloud_ptr->header.stamp).seconds());

// test separated transformed cloud
std::array<Eigen::Vector3f, 10> expected_top_pointcloud = {
{Eigen::Vector3f(10.0f, 0.0f, 0.0f), Eigen::Vector3f(0.0f, 10.0f, 0.0f),
Eigen::Vector3f(0.0f, 0.0f, 10.0f)}};
std::array<Eigen::Vector3f, 10> expected_left_pointcloud = {
{Eigen::Vector3f(10.0f, 0.0f, 0.0f), Eigen::Vector3f(0.0f, 10.0f, 0.0f),
Eigen::Vector3f(0.0f, 0.0f, 10.0f)}};
std::array<Eigen::Vector3f, 10> expected_right_pointcloud = {
{Eigen::Vector3f(10.0f, 0.0f, 0.0f), Eigen::Vector3f(0.0f, 10.0f, 0.0f),
Eigen::Vector3f(0.0f, 0.0f, 10.0f)}};

oss.clear();
oss.str("");
i = 0;

sensor_msgs::PointCloud2Iterator<float> top_pc_iter_x(
*(topic_to_transformed_cloud_map.value().at("lidar_top")), "x");
sensor_msgs::PointCloud2Iterator<float> top_pc_iter_y(
*(topic_to_transformed_cloud_map.value().at("lidar_top")), "y");
sensor_msgs::PointCloud2Iterator<float> top_pc_iter_z(
*(topic_to_transformed_cloud_map.value().at("lidar_top")), "z");

for (; top_pc_iter_x != top_pc_iter_x.end();
++top_pc_iter_x, ++top_pc_iter_y, ++top_pc_iter_z, ++i) {
oss << "Top point " << i << ": (" << *top_pc_iter_x << ", " << *top_pc_iter_y << ", "
<< *top_pc_iter_z << ")\n";
EXPECT_FLOAT_EQ(*top_pc_iter_x, expected_top_pointcloud[i].x());
EXPECT_FLOAT_EQ(*top_pc_iter_y, expected_top_pointcloud[i].y());
EXPECT_FLOAT_EQ(*top_pc_iter_z, expected_top_pointcloud[i].z());
}

if (debug_) {
RCLCPP_INFO(concatenate_node_->get_logger(), "%s", oss.str().c_str());
}

oss.clear();
oss.str("");
i = 0;
sensor_msgs::PointCloud2Iterator<float> left_pc_iter_x(
*(topic_to_transformed_cloud_map.value().at("lidar_left")), "x");
sensor_msgs::PointCloud2Iterator<float> left_pc_iter_y(
*(topic_to_transformed_cloud_map.value().at("lidar_left")), "y");
sensor_msgs::PointCloud2Iterator<float> left_pc_iter_z(
*(topic_to_transformed_cloud_map.value().at("lidar_left")), "z");

for (; left_pc_iter_x != left_pc_iter_x.end();
++left_pc_iter_x, ++left_pc_iter_y, ++left_pc_iter_z, ++i) {
oss << "Left point " << i << ": (" << *left_pc_iter_x << ", " << *left_pc_iter_y << ", "
<< *left_pc_iter_z << ")\n";
EXPECT_FLOAT_EQ(*left_pc_iter_x, expected_left_pointcloud[i].x());
EXPECT_FLOAT_EQ(*left_pc_iter_y, expected_left_pointcloud[i].y());
EXPECT_FLOAT_EQ(*left_pc_iter_z, expected_left_pointcloud[i].z());
}

if (debug_) {
RCLCPP_INFO(concatenate_node_->get_logger(), "%s", oss.str().c_str());
}

oss.clear();
oss.str("");
i = 0;
sensor_msgs::PointCloud2Iterator<float> right_pc_iter_x(
*(topic_to_transformed_cloud_map.value().at("lidar_right")), "x");
sensor_msgs::PointCloud2Iterator<float> right_pc_iter_y(
*(topic_to_transformed_cloud_map.value().at("lidar_right")), "y");
sensor_msgs::PointCloud2Iterator<float> right_pc_iter_z(
*(topic_to_transformed_cloud_map.value().at("lidar_right")), "z");

for (; right_pc_iter_x != right_pc_iter_x.end();
++right_pc_iter_x, ++right_pc_iter_y, ++right_pc_iter_z, ++i) {
oss << "Right point " << i << ": (" << *right_pc_iter_x << ", " << *right_pc_iter_y << ", "
<< *right_pc_iter_z << ")\n";
EXPECT_FLOAT_EQ(*right_pc_iter_x, expected_right_pointcloud[i].x());
EXPECT_FLOAT_EQ(*right_pc_iter_y, expected_right_pointcloud[i].y());
EXPECT_FLOAT_EQ(*right_pc_iter_z, expected_right_pointcloud[i].z());
}

if (debug_) {
RCLCPP_INFO(concatenate_node_->get_logger(), "%s", oss.str().c_str());
}

// test original cloud's timestamps
EXPECT_FLOAT_EQ(top_timestamp.seconds(), topic_to_original_stamp_map["lidar_top"]);
EXPECT_FLOAT_EQ(left_timestamp.seconds(), topic_to_original_stamp_map["lidar_left"]);
EXPECT_FLOAT_EQ(right_timestamp.seconds(), topic_to_original_stamp_map["lidar_right"]);
}

Check warning on line 474 in sensing/autoware_pointcloud_preprocessor/test/test_concatenate_node_unit.cpp

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Complex Method

TEST:ConcatenateCloudTest:TestConcatenateClouds has a cyclomatic complexity of 9, 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.

TEST_F(ConcatenateCloudTest, TestProcessSingleCloud)
{
Expand Down
Loading