feat(reaction_analyzer): add reaction anaylzer tool to measure end-to-end delay in sudden obstacle braking response

[brkay54]

Description

Main issue:

• https://github.com/autowarefoundation/autoware.universe/issues/6547

Strictly depends on:

• https://github.com/autowarefoundation/autoware.universe/pull/6382

Optionally depends on:

• https://github.com/autowarefoundation/autoware.universe/issues/6255

The main purpose of the reaction analyzer package is to measure the reaction times of the nodes by listening to the pre-determined topics. It can be used for measuring the reaction time of the perception pipeline, planning pipeline, and control pipeline. To be able to measure both control outputs and perception outputs, it is necessary to divide the node into two running_mode: planning_control and perception_planning.

Planning Control Mode

In this mode, the reaction analyzer creates a dummy publisher for the PredictedObjects and PointCloud topics. In the beginning of the test, it publishes the initial position of the ego vehicle and the goal position to set the test environment. Then, it spawns a sudden obstacle in front of the ego vehicle. After the obstacle is spawned, it starts to measure the pre-determined topics to catch the reacted messages of the planning and control nodes. When all the topics are reacted, it calculates the reaction time of the nodes and statistics, and it creates a csv file to store the results.

Perception Planning Mode

In this mode, the reaction analyzer reads the rosbag files which are recorded from AWSIM, and it creates a topic publisher for each topic to replay the rosbag. It reads two rosbag files: path_bag_without_object and path_bag_with_object. Firstly, it replays the path_bag_without_object to set the initial position of the ego vehicle and the goal position. After spawn_time_after_init seconds, it replays the path_bag_with_object to spawn a sudden obstacle in front of the ego vehicle. After the obstacle is spawned, it starts to measure the pre-determined topics to catch the reacted messages of the perception and planning nodes. When all the topics are reacted, it calculates the reaction time of the nodes and statistics, and it creates a CSV file to store the results.

Related links

Related Issue:

• https://github.com/autowarefoundation/autoware.universe/issues/5540

Tests performed

You can see the results in the Excel table here. You can see the test dates below the page. Also, some tests were shared in issues comments: https://github.com/autowarefoundation/autoware.universe/issues/5540

Notes for reviewers

This PR currently depends on other PR below:

• https://github.com/autowarefoundation/autoware.universe/pull/6382

Interface changes

Effects on system behavior

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

• [x] I've confirmed the contribution guidelines.
• [x] The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

• [ ] The PR follows the pull request guidelines.
• [ ] The PR has been properly tested.
• [ ] The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

• [ ] There are no open discussions or they are tracked via tickets.
• [ ] The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

[brkay54]

cc @mitsudome-r @xmfcx The blockers are ready for review. I added a class utility to enable or disable easily the published_time:

by order of merge

• https://github.com/autowarefoundation/autoware_msgs/pull/83
• https://github.com/autowarefoundation/autoware.universe/pull/6440
• https://github.com/autowarefoundation/autoware.universe/pull/6490

• For the async pointcloud publishing, I added a parameter pointcloud_publisher_type which can be "async_header_sync_publish", "sync_header_sync_publish" or "async_publish". I tested again but I couldn't any better results for "async_header_sync_publish", "sync_header_sync_publish" or "async_publish".

[brkay54]

Sorry for the delay, I need to convert the PR because of the implementation of the message_filters package.

When I implemented message_filters into my node, the subscribers and synchronizers did not create their subscribers into the node but they were initialized. After some time, I realized the Class structure of my node does not allow me to create message_filters object when I try to register the callback function. So, I decided to change the structure and I added another class SubscriberBase.

The package is ready for review now. I set up the message_filters package into reaction_analyzer and cleaned up the code.

This PR currently depends on other PRs below(in order):

• https://github.com/autowarefoundation/autoware.universe/pull/6440
• https://github.com/autowarefoundation/autoware.universe/pull/6490 (not strictly)
• https://github.com/autowarefoundation/autoware.universe/pull/6382

[xmfcx]

@brkay54 could you fix the errors stated in the pre-commit.ci?

[brkay54]

@xmfcx Thank you, I fixed.

[coderabbitai[bot]]

[!IMPORTANT]

Auto Review Skipped

Auto reviews are limited to the following labels: tag:openai-pr-reviewer. Please add one of these labels to enable auto reviews.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[github-actions[bot]]

CodeRabbit

---

Uplevel your code reviews with CodeRabbit Pro

CodeRabbit Pro

If you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects.

[xmfcx]

/review

[github-actions[bot]]

PR Review

(Review updated until commit https://github.com/autowarefoundation/autoware.universe/commit/3e8a41f8ecbfca3a04511f57d4bfdf3ab141e08b)

⏱️ Estimated effort to review [1-5]	4, due to the large amount of new code across multiple files, complexity of the logic involved, and the need to understand the context of the Autoware system and its message types.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: In reaction_analyzer_node.cpp, the method spawn_obstacle checks if spawn_object_cmd_ is false before setting it to true and logging that the spawn command is sent. However, there's no mechanism to ensure that the obstacle is only spawned once or to handle cases where the spawning might fail or need to be retried.
	Performance Concern: The use of mutex locks in multiple callback functions in reaction_analyzer_node.cpp might lead to performance bottlenecks, especially if these callbacks are called frequently. It's important to ensure that the locking is absolutely necessary and that the scope of the lock is as minimal as possible.
🔒 Security concerns	No

Code feedback:

---

relevant file	tools/reaction_analyzer/src/reaction_analyzer_node.cpp
suggestion	Consider adding a mechanism to ensure that the obstacle is only spawned once and handle retry logic if spawning fails. This could involve setting a flag once the obstacle is successfully spawned and checking this flag before attempting to spawn again. [important]
relevant line	if (!spawn_object_cmd_) {

---

relevant file	tools/reaction_analyzer/src/reaction_analyzer_node.cpp
suggestion	Review the use of mutex locks in callback functions to ensure they are necessary and minimize the scope of the locks to avoid potential performance bottlenecks. [medium]
relevant line	std::lock_guard<:mutex> lock(mutex_);

---

relevant file	tools/reaction_analyzer/src/utils.cpp
suggestion	For functions like calculate_statistics, consider passing the input vector by const reference instead of value to avoid unnecessary copying, which can improve performance especially for large datasets. [medium]
relevant line	LatencyStats calculate_statistics(const std::vector & latency_vec)

---

relevant file	tools/reaction_analyzer/src/topic_publisher.cpp
suggestion	In the init_rosbag_publishers method, when handling exceptions for opening bag files, consider adding more specific error handling than just logging and shutting down. This could include retry logic or fallback mechanisms. [medium]
relevant line	RCLCPP_ERROR_STREAM(node_->get_logger(), "Error opening bag file: "

---

---

✨ Review tool usage guide:

---

Overview: The review tool scans the PR code changes, and generates a PR review. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR. When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

Utilizing extra instructions The review tool can be configured with extra instructions, which can be used to guide the model to a feedback tailored to the needs of your project. Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Specify the relevant sub-tool, and the relevant aspects of the PR that you want to emphasize. Examples for extra instructions: [pr_reviewer] # /review # extra_instructions=""" In the 'possible issues' section, emphasize the following: - Does the code logic cover relevant edge cases? - Is the code logic clear and easy to understand? - Is the code logic efficient? ... """ Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable.

How to enable\disable automation When you first install PR-Agent app, the default mode for the review tool is: pr_commands = ["/review", ...] meaning the review tool will run automatically on every PR, with the default configuration. Edit this field to enable/disable the tool, or to change the used configurations

Auto-labels The review tool can auto-generate two specific types of labels for a PR: a possible security issue label, that detects possible security issues (enable_review_labels_security flag) a Review effort [1-5]: x label, where x is the estimated effort to review the PR (enable_review_labels_effort flag)

Extra sub-tools The review tool provides a collection of possible feedbacks about a PR. It is recommended to review the possible options, and choose the ones relevant for your use case. Some of the feature that are disabled by default are quite useful, and should be considered for enabling. For example: require_score_review, require_soc2_ticket, require_can_be_split_review, and more.

Auto-approve PRs By invoking: /review auto_approve The tool will automatically approve the PR, and add a comment with the approval. To ensure safety, the auto-approval feature is disabled by default. To enable auto-approval, you need to actively set in a pre-defined configuration file the following: [pr_reviewer] enable_auto_approval = true (this specific flag cannot be set with a command line argument, only in the configuration file, committed to the repository) You can also enable auto-approval only if the PR meets certain requirements, such as that the estimated_review_effort is equal or below a certain threshold, by adjusting the flag: [pr_reviewer] maximal_review_effort = 5

More PR-Agent commands To invoke the PR-Agent, add a comment using one of the following commands: /review: Request a review of your Pull Request. /describe: Update the PR title and description based on the contents of the PR. /improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback. /ask <QUESTION>: Ask a question about the PR. /update_changelog: Update the changelog based on the PR's contents. /add_docs 💎: Generate docstring for new components introduced in the PR. /generate_labels 💎: Generate labels for the PR based on the PR's contents. /analyze 💎: Automatically analyzes the PR, and presents changes walkthrough for each component. See the tools guide for more details. To list the possible configuration parameters, add a /config comment.

See the review usage page for a comprehensive guide on using this tool.

[xmfcx]

/improve --extended

[github-actions[bot]]

PR Code Suggestions

Category	Suggestions
Best practice	Use std::atomic for thread-safe boolean flags. Consider using std::atomic for the boolean flags is_vehicle_initialized_ and is_route_set_ to ensure thread-safe read and write operations, especially since these flags might be accessed or modified from multiple threads. tools/reaction_analyzer/include/reaction_analyzer_node.hpp [157-158] -bool is_vehicle_initialized_{false}; -bool is_route_set_{false}; +std::atomic<bool> is_vehicle_initialized_{false}; +std::atomic<bool> is_route_set_{false};
	Use smart pointers for better resource management. To avoid potential memory leaks and ensure proper resource management, consider using smart pointers (std::unique_ptr or std::shared_ptr) instead of raw pointers for topic_publisher_ptr_ and subscriber_ptr_. tools/reaction_analyzer/include/reaction_analyzer_node.hpp [197-198] -std::shared_ptr<topic_publisher::TopicPublisher> topic_publisher_ptr_; +std::unique_ptr<topic_publisher::TopicPublisher> topic_publisher_ptr_; std::unique_ptr<subscriber::SubscriberBase> subscriber_ptr_;
	Delete copy constructor and assignment operator for classes with mutexes. Since mutex_ is used to protect shared data in a multithreaded context, it's a good practice to explicitly delete the copy constructor and copy assignment operator of the SubscriberBase class to prevent accidental copying of mutexes, which is unsafe. tools/reaction_analyzer/include/subscriber.hpp [171] -std::mutex mutex_; +SubscriberBase(const SubscriberBase&) = delete; +SubscriberBase& operator=(const SubscriberBase&) = delete;
	Use std::chrono types for time duration representation. Consider using std::chrono types for time duration representation instead of double for period_ns in PublisherVariables struct. This change enhances type safety and readability. tools/reaction_analyzer/include/topic_publisher.hpp [98] -double period_ns{0.0}; +std::chrono::nanoseconds period{0};
	Use smart pointers for better memory management. To avoid potential memory leaks and ensure exception safety, consider using std::unique_ptrrclcpp::Node instead of a raw pointer for the node parameter in create_subscription_options. tools/reaction_analyzer/include/utils.hpp [50] -rclcpp::SubscriptionOptions create_subscription_options(rclcpp::Node * node); +rclcpp::SubscriptionOptions create_subscription_options(std::unique_ptr<rclcpp::Node>& node);
	Set use_sim_time to true for simulation time synchronization. To ensure that the simulation time is correctly synchronized with the ROS 2 system, especially when running simulations, it is advisable to set use_sim_time to true. This ensures that all nodes in the simulation use the simulation clock. tools/reaction_analyzer/launch/reaction_analyzer.launch.xml [25] -<arg name="use_sim_time" value="false"/> +<arg name="use_sim_time" value="true"/>
	Replace hardcoded file paths with environment variables for enhanced portability. It's recommended to avoid using hardcoded file paths in configuration files. Instead, consider using environment variables or configuration management systems to dynamically set these paths. This approach enhances portability and flexibility across different environments. tools/reaction_analyzer/param/reaction_analyzer.param.yaml [5-37] -output_file_path: <PATH>/reaction_analyzer_results/ -path_bag_without_object: <PATH_TO_YOUR_BAGS>/lexus_bag_without_car/rosbag2_2024_01_30-13_50_45_0.db3 -path_bag_with_object: <PATH_TO_YOUR_BAGS>/lexus_bag_with_car/rosbag2_2024_01_30-13_50_17_0.db3 +output_file_path: ${REACTION_ANALYZER_RESULTS_PATH} +path_bag_without_object: ${PATH_TO_YOUR_BAGS}/lexus_bag_without_car/rosbag2_2024_01_30-13_50_45_0.db3 +path_bag_with_object: ${PATH_TO_YOUR_BAGS}/lexus_bag_with_car/rosbag2_2024_01_30-13_50_17_0.db3
Maintainability	Move struct definitions to separate header files for better organization. To improve the maintainability and readability of the code, consider moving the definitions of PoseParams, EntityParams, and NodeParams structures into separate header files. This can help in organizing the code better and make it easier to manage. tools/reaction_analyzer/include/reaction_analyzer_node.hpp [75-110] +// In a separate header file, e.g., PoseParams.hpp struct PoseParams { double x; double y; double z; double roll; double pitch; double yaw; };
	Improve encapsulation by moving methods into PublisherVariables. For better encapsulation and data integrity, consider making the PublisherVarAccessor methods that modify PublisherVariables (set_period, publish_with_current_time) member functions of the PublisherVariables struct itself. tools/reaction_analyzer/include/topic_publisher.hpp [153-155] -void set_period(T & publisherVar, double newPeriod) +void set_period(double newPeriod) { this->period_ns = newPeriod; }
	Split TopicPublisher into smaller, more focused classes. To reduce the complexity and improve readability, consider splitting the TopicPublisher class into smaller classes, each responsible for handling a specific type of message or functionality. tools/reaction_analyzer/include/topic_publisher.hpp [194-242] -class TopicPublisher +class PointCloudPublisher +class ImagePublisher +// Other specific publishers
	Add a setup script to automate the environment setup process. For better reproducibility and ease of use, consider adding a script or automation tool that can handle the setup process, including downloading necessary files, checking out specific branches, and setting up the test environment. This would streamline the setup process for users. tools/reaction_analyzer/README.md [58-61] -Download the demonstration test map from the link [here](https://github.com/tier4/AWSIM/releases/download/v1.1.0/nishishinjuku_autoware_map.zip). After downloading, extract the zip file and use its path as `[MAP_PATH]` in the following commands. +To simplify the setup process, we provide a setup script that automates the following steps: +1. Downloading the demonstration test map. +2. Extracting the zip file. +3. Setting up the necessary environment variables and configurations. +Run the following command to execute the setup script: `./setup_reaction_analyzer.sh`
	Group related parameters under a common hierarchy for better organization. For better readability and maintainability, consider grouping related parameters under a common hierarchy. For example, initialization_pose, entity_params, and goal_pose could be grouped under a poses hierarchy. This makes the configuration file more organized and easier to navigate. tools/reaction_analyzer/param/reaction_analyzer.param.yaml [11-34] -initialization_pose: - x: 81247.9609375 - y: 49828.87890625 - z: 0.0 - roll: 0.0 - pitch: 0.0 - yaw: 35.5 -entity_params: - x: 81392.97671487389 - y: 49927.361443601316 - z: 42.13605499267578 - roll: 0.2731273 - pitch: -0.6873504 - yaw: 33.7664809 - x_dimension: 4.118675972722859 - y_dimension: 1.7809072588403219 - z_dimension: 0.8328610206872963 -goal_pose: - x: 81824.90625 - y: 50069.34375 - z: 0.0 - roll: 0.0 - pitch: 0.0 - yaw: 8.9305903 +poses: + initialization_pose: + x: 81247.9609375 + y: 49828.87890625 + z: 0.0 + roll: 0.0 + pitch: 0.0 + yaw: 35.5 + entity_params: + x: 81392.97671487389 + y: 49927.361443601316 + z: 42.13605499267578 + roll: 0.2731273 + pitch: -0.6873504 + yaw: 33.7664809 + x_dimension: 4.118675972722859 + y_dimension: 1.7809072588403219 + z_dimension: 0.8328610206872963 + goal_pose: + x: 81824.90625 + y: 50069.34375 + z: 0.0 + roll: 0.0 + pitch: 0.0 + yaw: 8.9305903
Enhancement	Abstract subscriber logic into a template class for modularity. To enhance the modularity and reusability of the subscriber components, consider abstracting the subscriber logic into a template class that can be instantiated with different message types. This approach can reduce code duplication and make the system more flexible to changes. tools/reaction_analyzer/include/subscriber.hpp [96-99] -using SubscriberVariablesVariant = std::variant< - SubscriberVariables<PointCloud2>, SubscriberVariables<DetectedObjects>, - SubscriberVariables<TrackedObjects>, SubscriberVariables<PredictedObjects>, - SubscriberVariables<Trajectory>, SubscriberVariables<AckermannControlCommand>>; +template<typename MessageType> +class SubscriberTemplate +{ + // Template class implementation +};
	Provide a detailed setup and usage guide for the reaction_analyzer tool. To enhance the clarity and maintainability of the documentation, consider providing a more detailed explanation or step-by-step guide on how to set up and use the reaction_analyzer tool, especially for users who might be new to ROS or the specific simulation environment being used. tools/reaction_analyzer/README.md [5] -The main purpose of the reaction analyzer package is measuring reaction times of the nodes by listening the pre-determined topics. +The main purpose of the reaction analyzer package is to measure the reaction times of various nodes within a ROS-based autonomous driving simulation environment by subscribing to pre-determined topics. This tool is particularly useful for evaluating the performance of perception, planning, and control pipelines in response to dynamic changes in the environment, such as sudden obstacles. Here's a step-by-step guide on how to set up and use the `reaction_analyzer`:
	Host documentation images on a stable and accessible platform. To improve the usability and accessibility of the documentation, consider hosting the images (ReactionAnalyzerDesign.png and PointcloudPublisherType.png) on a more stable and accessible platform such as GitHub or a documentation website. This ensures that users can view these images even if they have firewall restrictions or limited access to external media hosting sites. tools/reaction_analyzer/README.md [10] - +
Performance	Improve median calculation efficiency using std::nth_element. Instead of manually calculating the median, consider using the std::nth_element algorithm to find the median. This approach can potentially improve the efficiency of the median calculation. tools/reaction_analyzer/src/utils.cpp [37-45] -std::sort(vec.begin(), vec.end()); -double median = 0.0; size_t size = vec.size(); +size_t mid_index = size / 2; +std::nth_element(vec.begin(), vec.begin() + mid_index, vec.end()); +double median = vec[mid_index]; if (size % 2 == 0) { - median = (vec[size / 2 - 1] + vec[size / 2]) / 2.0; -} else { - median = vec[size / 2]; + auto max_it = std::max_element(vec.begin(), vec.begin() + mid_index); + median = (*max_it + median) / 2.0; }
	Enable intra-process communication for the node container. It is recommended to enable intra-process communication for the node container to improve performance by reducing message passing overhead. This is particularly beneficial in a component-based architecture where nodes may frequently exchange messages. tools/reaction_analyzer/launch/reaction_analyzer.launch.xml [65] -<extra_arg name="use_intra_process_comms" value="false"/> +<extra_arg name="use_intra_process_comms" value="true"/>

---

✨ Improve tool usage guide:

---

Overview: The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR. When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

Enabling\disabling automation When you first install the app, the default mode for the improve tool is: pr_commands = ["/improve --pr_code_suggestions.summarize=true", ...] meaning the improve tool will run automatically on every PR, with summarization enabled. Delete this line to disable the tool from running automatically.

Utilizing extra instructions Extra instructions are very important for the improve tool, since they enable to guide the model to suggestions that are more relevant to the specific needs of the project. Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Specify relevant aspects that you want the model to focus on. Examples for extra instructions: [pr_code_suggestions] # /improve # extra_instructions=""" Emphasize the following aspects: - Does the code logic cover relevant edge cases? - Is the code logic clear and easy to understand? - Is the code logic efficient? ... """ Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable.

A note on code suggestions quality While the current AI for code is getting better and better (GPT-4), it's not flawless. Not all the suggestions will be perfect, and a user should not accept all of them automatically. Suggestions are not meant to be simplistic. Instead, they aim to give deep feedback and raise questions, ideas and thoughts to the user, who can then use his judgment, experience, and understanding of the code base. Recommended to use the 'extra_instructions' field to guide the model to suggestions that are more relevant to the specific needs of the project, or use the custom suggestions :gem: tool With large PRs, best quality will be obtained by using 'improve --extended' mode.

More PR-Agent commands To invoke the PR-Agent, add a comment using one of the following commands: /review: Request a review of your Pull Request. /describe: Update the PR title and description based on the contents of the PR. /improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback. /ask <QUESTION>: Ask a question about the PR. /update_changelog: Update the changelog based on the PR's contents. /add_docs 💎: Generate docstring for new components introduced in the PR. /generate_labels 💎: Generate labels for the PR based on the PR's contents. /analyze 💎: Automatically analyzes the PR, and presents changes walkthrough for each component. See the tools guide for more details. To list the possible configuration parameters, add a /config comment.

See the improve usage page for a more comprehensive guide on using this tool.

[brkay54]

/review

[github-actions[bot]]

Persistent review updated to latest commit https://github.com/autowarefoundation/autoware.universe/commit/3e8a41f8ecbfca3a04511f57d4bfdf3ab141e08b

[brkay54]

/improve --extended

[github-actions[bot]]

PR Code Suggestions

Category	Suggestions
Best practice	Initialize primitive type members in structs with default values. Consider initializing all primitive type members in the NodeParams struct directly within the struct definition to ensure they have default values. This can prevent uninitialized variable issues and make the code safer and more predictable. tools/reaction_analyzer/include/reaction_analyzer_node.hpp [53-63] struct NodeParams { std::string running_mode; - double timer_period; + double timer_period = 0.0; std::string output_file_path; - size_t test_iteration; - double spawn_time_after_init; - double spawn_distance_threshold; + size_t test_iteration = 0; + double spawn_time_after_init = 0.0; + double spawn_distance_threshold = 0.0; PoseParams initial_pose; PoseParams goal_pose; EntityParams entity_params; };
	Encapsulate public member variables with getter functions. For better encapsulation and data protection, consider making the odometry_ptr_ member private and provide a public getter function for accessing its value. This follows the principle of least privilege and encapsulation in object-oriented design. tools/reaction_analyzer/include/reaction_analyzer_node.hpp [72] -Odometry::ConstSharedPtr odometry_ptr_; +private: + Odometry::ConstSharedPtr odometry_ptr_; +public: + Odometry::ConstSharedPtr getOdometryPtr() const { return odometry_ptr_; }
	Use smart pointers for better resource management. To avoid potential memory leaks and ensure proper resource management, consider using smart pointers (std::shared_ptr or std::unique_ptr) instead of raw pointers for tf_buffer_ and tf_listener_. This change will leverage RAII (Resource Acquisition Is Initialization) for automatic resource management. tools/reaction_analyzer/include/subscriber.hpp [173-174] -std::shared_ptr<tf2_ros::Buffer> tf_buffer_; -std::shared_ptr<tf2_ros::TransformListener> tf_listener_; +std::unique_ptr<tf2_ros::Buffer> tf_buffer_; +std::unique_ptr<tf2_ros::TransformListener> tf_listener_;
	Use std::make_shared for more efficient memory allocation when creating shared pointers. Consider using std::make_shared instead of std::shared_ptr with new for creating shared pointers. This is more efficient as it performs a single memory allocation, compared to two allocations when using std::shared_ptr with new. tools/reaction_analyzer/include/topic_publisher.hpp [218-219] -std::shared_ptr<tf2_ros::Buffer> tf_buffer_; -std::shared_ptr<tf2_ros::TransformListener> tf_listener_; +auto tf_buffer_ = std::make_shared<tf2_ros::Buffer>(); +auto tf_listener_ = std::make_shared<tf2_ros::TransformListener>(tf_buffer_);
	Make PublisherVarAccessor methods static for better encapsulation. For better encapsulation and data hiding, consider making the PublisherVarAccessor methods static if they do not need to access any instance variables. This way, they can be called without needing an instance of PublisherVarAccessor. tools/reaction_analyzer/include/topic_publisher.hpp [133-151] -void publish_with_current_time( +static void publish_with_current_time( const PublisherVariables<MessageType> & publisherVar, const rclcpp::Time & current_time, - const bool is_object_spawned) const + const bool is_object_spawned)
	Consider adding iteration capability to enum class if needed. It's recommended to use enum class over plain enum for type safety and scoping. However, if you need to iterate over an enum class, consider defining a helper function or operator to enable this functionality. tools/reaction_analyzer/include/topic_publisher.hpp [58-73] -enum class PublisherMessageType { - UNKNOWN = 0, - CAMERA_INFO = 1, - ... -}; +// No direct code change suggested here, but consider implementing a helper function for iteration if needed.
	Use smart pointers instead of raw pointers for better memory management. To avoid potential memory leaks and ensure exception safety, consider using smart pointers instead of raw pointers. If dynamic allocation is not necessary, consider using stack allocation. tools/reaction_analyzer/include/utils.hpp [185-190] -rclcpp::SubscriptionOptions create_subscription_options(rclcpp::Node * node); +rclcpp::SubscriptionOptions create_subscription_options(std::shared_ptr<rclcpp::Node> node);
	Mark functions as const or static where applicable for better code clarity. For functions that do not modify member variables or only access static members, consider declaring them as const or static to improve code clarity and enforce const-correctness. tools/reaction_analyzer/include/utils.hpp [134-138] -LatencyStats calculate_statistics(const std::vector<double> & latency_vec); +static LatencyStats calculate_statistics(const std::vector<double> & latency_vec);
	Use RAII locking mechanisms for mutexes to ensure exception safety. Instead of manually locking and unlocking the mutex, consider using std::lock_guard or std::scoped_lock for exception safety and cleaner code. tools/reaction_analyzer/src/reaction_analyzer_node.cpp [152-159] -mutex_.lock(); +std::lock_guard<std::mutex> lock(mutex_); const auto current_odometry_ptr = odometry_ptr_; const auto initialization_state_ptr = initialization_state_ptr_; const auto route_state_ptr = current_route_state_ptr_; const auto operation_mode_ptr = operation_mode_ptr_; const auto ground_truth_pose_ptr = ground_truth_pose_ptr_; const auto spawn_cmd_time = spawn_cmd_time_; -mutex_.unlock();
	Use atomic variables for thread-safe flag operations. To ensure thread safety, consider using atomic variables for flags such as is_vehicle_initialized_ and is_route_set_ instead of plain bools, especially if they can be accessed from multiple threads. tools/reaction_analyzer/src/reaction_analyzer_node.cpp [375-376] -is_vehicle_initialized_ = false; -is_route_set_ = false; +std::atomic<bool> is_vehicle_initialized_{false}; +std::atomic<bool> is_route_set_{false};
Enhancement	Consider using std::recursive_mutex for complex locking scenarios. Replace std::mutex with std::recursive_mutex if you need to lock the mutex multiple times in the same thread, which can happen in complex callback scenarios. If not, ensure that your current usage of std::mutex does not lead to deadlocks by carefully managing lock and unlock operations. tools/reaction_analyzer/include/reaction_analyzer_node.hpp [75] -std::mutex mutex_; +std::recursive_mutex mutex_;
	Use std::chrono::duration for time difference calculation. For better accuracy and to avoid precision loss, consider using std::chrono::duration<double, std::milli> directly in calculate_time_diff_ms instead of converting to nanoseconds and then dividing. tools/reaction_analyzer/src/utils.cpp [62-63] -const auto duration_ns = duration.to_chrono<std::chrono::nanoseconds>(); -return static_cast<double>(duration_ns.count()) / 1e6; +const auto duration_ms = std::chrono::duration<double, std::milli>(duration); +return duration_ms.count();
	Add check for empty input string in UUID generation. In generate_uuid_msg, consider checking if the input string is empty and handle it accordingly to avoid generating UUIDs for empty strings unintentionally. tools/reaction_analyzer/src/utils.cpp [95] +if (input.empty()) throw std::invalid_argument("Input string for UUID generation is empty."); const auto uuid = generate_uuid(input);
	Add mutex to ensure thread safety in UUID generation. To ensure thread safety, consider protecting the static generate_uuid function in generate_uuid_msg with a mutex, as static local variables in a function scope may not be thread-safe when modified. tools/reaction_analyzer/src/utils.cpp [94-95] +static std::mutex uuid_mutex; +std::lock_guard<std::mutex> lock(uuid_mutex); static auto generate_uuid = boost::uuids::name_generator(boost::uuids::random_generator()()); const auto uuid = generate_uuid(input);
Maintainability	Abstract complex initialization logic into a separate method for clarity. To improve code readability and maintainability, consider abstracting the complex initialization logic in SubscriberBase constructor into a separate, well-named method. This can make the constructor more concise and the overall class easier to understand. tools/reaction_analyzer/include/subscriber.hpp [140-142] explicit SubscriberBase( + rclcpp::Node * node, Odometry::ConstSharedPtr & odometry, std::atomic<bool> & spawn_object_cmd, + const EntityParams & entity_params) +{ + initializeSubscriberBase(node, odometry, spawn_object_cmd, entity_params); +} + +void initializeSubscriberBase( rclcpp::Node * node, Odometry::ConstSharedPtr & odometry, std::atomic<bool> & spawn_object_cmd, const EntityParams & entity_params);
	Encapsulate running mode logic into separate functions for clarity. For better maintainability and readability, consider encapsulating the logic for handling different running modes into separate functions or classes. This would make the code easier to understand and modify. tools/reaction_analyzer/src/reaction_analyzer_node.cpp [121-133] -if (node_running_mode_ == RunningMode::PlanningControl) { - pub_initial_pose_ = create_publisher<geometry_msgs::msg::PoseWithCovarianceStamped>( - "output/initialpose", rclcpp::QoS(1)); - client_change_to_autonomous_ = - create_client<ChangeOperationMode>("service/change_to_autonomous"); - } else if (node_running_mode_ == RunningMode::PerceptionPlanning) { - sub_ground_truth_pose_ = create_subscription<PoseStamped>( - "input/ground_truth_pose", rclcpp::QoS{1}, - std::bind(&ReactionAnalyzerNode::on_ground_truth_pose, this, _1), - create_subscription_options(this)); - } +initializeRunningMode(); +// Then define a function initializeRunningMode() that contains the conditional logic.
Performance	Use const reference for strings in loops to avoid unnecessary copies. To improve performance and avoid unnecessary copying, consider passing const std::string& instead of std::string for the key parameter in the loop iterating over message_buffers. tools/reaction_analyzer/src/reaction_analyzer_node.cpp [224-230] for (const auto & [key, variant] : message_buffers) { ReactionPair reaction_pair; if (auto * control_message = std::get_if<subscriber::ControlCommandBuffer>(&variant)) { if (control_message->second) { - reaction_pair.first = key; + reaction_pair.first = std::cref(key); reaction_pair.second = control_message->second.value(); } } }
	Reserve space for vector to improve performance. To improve performance, consider reserving space for elements vector in split function based on an estimated size to minimize reallocations. tools/reaction_analyzer/src/utils.cpp [32] std::vector<std::string> elements; +elements.reserve(10); // Adjust the reserved size based on expected input for better performance.
Possible issue	Add error handling for node shutdown process. Consider checking the return value of rclcpp::shutdown() for error handling, to ensure that the shutdown process is executed correctly. tools/reaction_analyzer/src/reaction_analyzer_node.cpp [313] -rclcpp::shutdown(); +if (!rclcpp::shutdown()) { + RCLCPP_ERROR(get_logger(), "Failed to shutdown the node."); +}
	Add bounds checking for curr_id before accessing traj.points. Consider checking if curr_id is within the bounds of traj.points before accessing it with at(curr_id) to prevent potential out-of-range errors. tools/reaction_analyzer/src/utils.cpp [44] +if (curr_id >= traj.points.size()) throw std::out_of_range("curr_id is out of range."); const TrajectoryPoint & curr_p = traj.points.at(curr_id);

---

✨ Improve tool usage guide:

---

Overview: The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR. When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

Enabling\disabling automation When you first install the app, the default mode for the improve tool is: pr_commands = ["/improve --pr_code_suggestions.summarize=true", ...] meaning the improve tool will run automatically on every PR, with summarization enabled. Delete this line to disable the tool from running automatically.

Utilizing extra instructions Extra instructions are very important for the improve tool, since they enable to guide the model to suggestions that are more relevant to the specific needs of the project. Be specific, clear, and concise in the instructions. With extra instructions, you are the prompter. Specify relevant aspects that you want the model to focus on. Examples for extra instructions: [pr_code_suggestions] # /improve # extra_instructions=""" Emphasize the following aspects: - Does the code logic cover relevant edge cases? - Is the code logic clear and easy to understand? - Is the code logic efficient? ... """ Use triple quotes to write multi-line instructions. Use bullet points to make the instructions more readable.

A note on code suggestions quality While the current AI for code is getting better and better (GPT-4), it's not flawless. Not all the suggestions will be perfect, and a user should not accept all of them automatically. Suggestions are not meant to be simplistic. Instead, they aim to give deep feedback and raise questions, ideas and thoughts to the user, who can then use his judgment, experience, and understanding of the code base. Recommended to use the 'extra_instructions' field to guide the model to suggestions that are more relevant to the specific needs of the project, or use the custom suggestions :gem: tool With large PRs, best quality will be obtained by using 'improve --extended' mode.

More PR-Agent commands To invoke the PR-Agent, add a comment using one of the following commands: /review: Request a review of your Pull Request. /describe: Update the PR title and description based on the contents of the PR. /improve [--extended]: Suggest code improvements. Extended mode provides a higher quality feedback. /ask <QUESTION>: Ask a question about the PR. /update_changelog: Update the changelog based on the PR's contents. /add_docs 💎: Generate docstring for new components introduced in the PR. /generate_labels 💎: Generate labels for the PR based on the PR's contents. /analyze 💎: Automatically analyzes the PR, and presents changes walkthrough for each component. See the tools guide for more details. To list the possible configuration parameters, add a /config comment.

See the improve usage page for a more comprehensive guide on using this tool.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 0% with 1248 lines in your changes are missing coverage. Please review.

Project coverage is 14.97%. Comparing base (010965b) to head (f74c83f). Report is 57 commits behind head on main.

Files	Patch %	Lines
tools/reaction_analyzer/src/subscriber.cpp	0.00%	493 Missing :warning:
tools/reaction_analyzer/src/utils.cpp	0.00%	295 Missing :warning:
tools/reaction_analyzer/src/topic_publisher.cpp	0.00%	240 Missing :warning:
...s/reaction_analyzer/src/reaction_analyzer_node.cpp	0.00%	207 Missing :warning:
...ools/reaction_analyzer/include/topic_publisher.hpp	0.00%	12 Missing :warning:
tools/reaction_analyzer/include/subscriber.hpp	0.00%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5954      +/-   ##
==========================================
- Coverage   15.01%   14.97%   -0.04%     
==========================================
  Files        1972     1969       -3     
  Lines      136512   136863     +351     
  Branches    42308    42030     -278     
==========================================
+ Hits        20494    20496       +2     
- Misses      93265    93658     +393     
+ Partials    22753    22709      -44     

Flag	Coverage Δ		*Carryforward flag
differential	0.00% <0.00%> (?)
total	15.11% <ø> (+0.10%)	:arrow_up:	Carriedforward from ac8d50b

*This pull request uses carry forward flags. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[brkay54]

@xmfcx I implemented some of the automated-review suggestions, cleaned-up the code, and changed .csv output: Before .csv output:

After .csv output:

• NL means node latency, TL means total latency

With the commit https://github.com/autowarefoundation/autoware.universe/pull/5954/commits/3e8a41f8ecbfca3a04511f57d4bfdf3ab141e08b, we can see each pipeline and its latencies by using the header time of the PublishedTime.msg. By using this, we can see if there are dropped messages or not while test runng.

Could you check the PR when you are available?

[xmfcx]

@brkay54 could you fix the clang-tidy errors?

• https://github.com/autowarefoundation/autoware.universe/actions/runs/8326653509/job/22782747570?pr=5954#step:8:7763

[brkay54]

@xmfcx Clang-tidy issues were fixed ⭐

[mitsudome-r]

I will test this PR against the main branch and merge it.

[mitsudome-r]

It seems my pointcloud_container keeps crashing when I launch planning_control mode. I will take a deeper look and see what's causing the error.

[brkay54]

It seems my pointcloud_container keeps crashing when I launch planning_control mode. I will take a deeper look and see what's causing the error.

Thank you @mitsudome-r -san, I am going to check this immidiately.

[brkay54]

@mitsudome-r -san, actually I saw this issue before but I didn't focus on that issue because it does not prevent running reaction_analyzer in planning control mode. Probably the error you encounter is similar.

Here log of the error:

[component_container_mt-1] [WARN 1715586401.197737367] [probabilistic_occupancy_grid_map]: Failed to lookup transform: Lookup would require extrapolation into the future.  Requested time 1715586400.691542 but the latest data is at time 1715586400.655299, when looking up transform from frame [base_link] to frame [map] (transformPointcloud() at /home/berkay/projects/autoware/src/universe/autoware.universe/perception/probabilistic_occupancy_grid_map/src/utils/utils.cpp:35)
[component_container_mt-1] terminate called after throwing an instance of 'std::runtime_error'
[component_container_mt-1]   what():  Field x does not exist
[component_container_mt-1] *** Aborted at 1715586401 (unix time) try "date -d @1715586401" if you are using GNU date ***
[component_container_mt-1] PC: @                0x0 (unknown)
[component_container_mt-1] *** SIGABRT (@0x3e8001bf6b6) received by PID 1832630 (TID 0x7f54e1ffb640) from PID 1832630; stack trace: ***
[component_container_mt-1]     @     0x7f550c40a006 google::(anonymous namespace)::FailureSignalHandler()
[component_container_mt-1]     @     0x7f5510842520 (unknown)
[component_container_mt-1]     @     0x7f55108969fc pthread_kill
[component_container_mt-1]     @     0x7f5510842476 raise
[component_container_mt-1]     @     0x7f55108287f3 abort
[component_container_mt-1]     @     0x7f5510ca2b9e (unknown)
[component_container_mt-1]     @     0x7f5510cae20c (unknown)
[component_container_mt-1]     @     0x7f5510cae277 std::terminate()
[component_container_mt-1]     @     0x7f5510cae4d8 __cxa_throw
[component_container_mt-1]     @     0x7f550470745b sensor_msgs::impl::PointCloud2IteratorBase<>::set_field()
[component_container_mt-1]     @     0x7f550470682b costmap_2d::OccupancyGridMap::raytraceFreespace()
[component_container_mt-1]     @     0x7f5504706d0b costmap_2d::OccupancyGridMap::raytrace2D()
[component_container_mt-1]     @     0x7f55046c19a6 occupancy_grid_map::LaserscanBasedOccupancyGridMapNode::onLaserscanPointCloud2WithObstacleAndRaw()
[component_container_mt-1]     @     0x7f55046ff00b message_filters::CallbackHelper9T<>::call()
[component_container_mt-1]     @     0x7f5504700da0 message_filters::sync_policies::ExactTime<>::checkTuple()
[component_container_mt-1]     @     0x7f550470267d message_filters::Synchronizer<>::cb<>()
[component_container_mt-1]     @     0x7f55046fdea8 message_filters::CallbackHelper1T<>::call()
[component_container_mt-1]     @     0x7f55046fe007 _ZNSt17_Function_handlerIFvSt10shared_ptrIKN11sensor_msgs3msg10LaserScan_ISaIvEEEEEZN15message_filters10SubscriberIS5_N6rclcpp4NodeEE9subscribeEPSC_RKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE17rmw_qos_profile_sNSB_32SubscriptionOptionsWithAllocatorIS4_EEEUlS7_E_E9_M_invokeERKSt9_Any_dataOS7_
[component_container_mt-1]     @     0x7f55046d6752 _ZNSt8__detail9__variant17__gen_vtable_implINS0_12_Multi_arrayIPFNS0_21__deduce_visit_resultIvEEOZN6rclcpp23AnySubscriptionCallbackIN11sensor_msgs3msg10LaserScan_ISaIvEEESA_E8dispatchESt10shared_ptrISB_ERKNS5_11MessageInfoEEUlOT_E_RSt7variantIJSt8functionIFvRKSB_EESN_IFvSP_SH_EESN_IFvRKNS5_17SerializedMessageEEESN_IFvSW_SH_EESN_IFvSt10unique_ptrISB_St14default_deleteISB_EEEESN_IFvS14_SH_EESN_IFvS11_ISU_S12_ISU_EEEESN_IFvS1A_SH_EESN_IFvSD_ISO_EEESN_IFvS1F_SH_EESN_IFvSD_ISV_EEESN_IFvS1K_SH_EESN_IFvRKS1F_EESN_IFvS1Q_SH_EESN_IFvRKS1K_EESN_IFvS1W_SH_EESN_IFvSE_EESN_IFvSE_SH_EESN_IFvSD_ISU_EEESN_IFvS25_SH_EEEEEJEEESt16integer_sequenceImJLm8EEEE14__visit_invokeESL_S2B_
[component_container_mt-1]     @     0x7f55046fb3d3 rclcpp::Subscription<>::handle_message()
[component_container_mt-1]     @     0x7f55110237bc rclcpp::Executor::execute_subscription()
[component_container_mt-1]     @     0x7f5511023fbf rclcpp::Executor::execute_any_executable()
[component_container_mt-1]     @     0x7f551102b27a rclcpp::executors::MultiThreadedExecutor::run()
[component_container_mt-1]     @     0x7f5510cdc253 (unknown)
[component_container_mt-1]     @     0x7f5510894ac3 (unknown)
[component_container_mt-1]     @     0x7f5510926850 (unknown)
[ERROR] [component_container_mt-1]: process has died [pid 1832630, exit code -6, cmd '/opt/ros/humble/lib/rclcpp_components/component_container_mt --ros-args -r __node:=pointcloud_container -r __ns:=/ -p use_sim_time:=False -p wheel_radius:=0.383 -p wheel_width:=0.235 -p wheel_base:=2.79 -p wheel_tread:=1.64 -p front_overhang:=1.0 -p rear_overhang:=1.1 -p left_overhang:=0.128 -p right_overhang:=0.128 -p vehicle_height:=2.5 -p max_steer_angle:=0.7'].

Let's discuss this at tomorrow's meeting.

[brkay54]

@mitsudome-r -san, I guess, I found the problem. When we start to publish the empty pointcloud before the initialization, it causes the crash above. We should publish the pointcloud message after the vehicle is initialized. I tried 7 times in planning_control mode and I didn't get the error we mentioned above. When you have time, could you test it again with the latest commit?

[beyzanurkaya]

Very nice work @brkay54 I tried this PR on my own computer and I think a few minor improvements can be made.

• Firstly, when running the planning_control mode without making any changes to the param file, including file paths, I noticed that even with invalid file paths in the param file, the node worked without giving any error or warning. You can throw an error when an invalid file path is given to make sure the user can see the results after testing.

• Secondly, in the scenario created for the planning_control mode, the vehicle starts driving and stops for an object close to the goal point, abruptly ending the test. However, since the object is far from the ego's starting point, it may not be immediately clear whether there is an object at that point until the vehicle moves forward for a while. This could lead to confusion about why the test ends at that specific point. To improve clarity, you can add a marker symbolizing the object there or start the next test a few seconds after the vehicle has stopped completely.

• When I ran the perception_planning mode (with no changes made to the param file other than the bag and output file paths), I encountered an issue where the test did not complete. Although the vehicle was initialized, the goal was set, and the route was generated with objects properly detected, the test did not proceed beyond this point. No output file was created. However, when I commented out the control topics in the yaml file and retried, the tests were completed successfully, and the result file was created. In this regard, it would be useful for users to specify the reaction topics that the mods expect separately or to add a warning about this to the readme.

Also, I did not encounter any node crashes like in the last comment. I think the README file was quite descriptive. Overall, it's a well-done piece of work!

[brkay54]

@beyzanurkaya Thank you for the review.

Firstly, when running the planning_control mode without making any changes to the param file, including file paths, I noticed that even with invalid file paths in the param file, the node worked without giving any error or warning. You can throw an error when an invalid file path is given to make sure the user can see the results after testing.

I have added a condition for checking whether the output path is valid or not at the beginning of the node.

Secondly, in the scenario created for the planning_control mode, the vehicle starts driving and stops for an object close to the goal point, abruptly ending the test. However, since the object is far from the ego's starting point, it may not be immediately clear whether there is an object at that point until the vehicle moves forward for a while. This could lead to confusion about why the test ends at that specific point. To improve clarity, you can add a marker symbolizing the object there or start the next test a few seconds after the vehicle has stopped completely.

Thank you for the suggestion. I added a debug marker for the entity that will be spawned.

When I ran the perception_planning mode (with no changes made to the param file other than the bag and output file paths), I encountered an issue where the test did not complete. Although the vehicle was initialized, the goal was set, and the route was generated with objects properly detected, the test did not proceed beyond this point. No output file was created. However, when I commented out the control topics in the yaml file and retried, the tests were completed successfully, and the result file was created. In this regard, it would be useful for users to specify the reaction topics that the mods expect separately or to add a warning about this to the readme.

Because the vehicle is not moving in perception_planning mode, the control topics may not react. Thus the reaction_analyzer waits for them to react. I added this warning in the readme, I also added a warning message prints reaction_analyzer is waiting for which node.

I appreciate your review. Could you also try with the last commit?