refactor(cie): restructure thread_configurator into ROS-param-based nodes#46
Open
atsushi421 wants to merge 4 commits intomainfrom
Open
refactor(cie): restructure thread_configurator into ROS-param-based nodes#46atsushi421 wants to merge 4 commits intomainfrom
atsushi421 wants to merge 4 commits intomainfrom
Conversation
…odes Port the structural changes from autowarefoundation/agnocast#1234: - Split prerun out into its own executable (prerun_node) with a dedicated main (prerun_node_main.cpp). ThreadConfiguratorNode and PrerunNode now both accept rclcpp::NodeOptions, making them usable as components. - Drop the bespoke --prerun / --config-file CLI. thread_configurator_node now reads the YAML path from the ROS parameter "config_file" and performs hardware validation / YAML load in its constructor, throwing std::runtime_error on failure. Rename the old src/main.cpp to src/thread_configurator_node_main.cpp for naming symmetry with prerun_node_main.cpp, and wrap its executor loop in try/catch so startup failures exit cleanly with code 1. - Add launch/thread_configurator.launch.xml with prerun (bool, default false) and config_file (string, default "") args, switching between prerun_node and thread_configurator_node via if/unless. - Install the launch directory under share/ and update the README with the new CLI and launch examples. Deadline-specific behavior (apply_deadline_configs / exist_deadline_config / std::cin.get) and the spin_once()+all_applied() loop are preserved. No backward compatibility is provided for the removed CLI flags. Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>
- Move "Loaded config from" log before hardware validation so the YAML path is still reported when validation fails. - Remove duplicate "Success: All of the configurations are applied." log that was printed twice when SCHED_DEADLINE configs existed. - Add explicit <stdexcept> / <vector> includes in thread_configurator_node.cpp. - Clarify launch-file description for the 'config_file' arg (required when prerun is false). - Document that ThreadConfiguratorNode's constructor may throw std::runtime_error in three distinct cases. Signed-off-by: atsushi421 <atsushi.yano.2@tier4.jp>
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors cie_thread_configurator to be ROS-parameter / launch-file driven by splitting the previous CLI-driven “prerun vs configurator” behavior into two dedicated executables and moving config selection to a config_file ROS parameter.
Changes:
- Split
prerun_nodeinto its own executable (prerun_node_main.cpp) and executable target. - Update
thread_configurator_nodeto load YAML from aconfig_fileROS parameter and perform hardware validation in-node. - Add an XML launch file to switch between
prerun_nodeandthread_configurator_node, and update install rules + README usage.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates usage examples to new executables and ROS parameter-based invocation. |
| cie_thread_configurator/src/thread_configurator_node.cpp | Moves YAML loading + hardware validation into the node constructor. |
| cie_thread_configurator/src/thread_configurator_node_main.cpp | New dedicated main for thread_configurator_node with exception handling. |
| cie_thread_configurator/src/prerun_node.cpp | Updates PrerunNode ctor to accept rclcpp::NodeOptions. |
| cie_thread_configurator/src/prerun_node_main.cpp | New dedicated main for prerun_node that dumps YAML on shutdown. |
| cie_thread_configurator/src/main.cpp | Removes old combined CLI-based entrypoint. |
| cie_thread_configurator/launch/thread_configurator.launch.xml | Adds canonical launch file with prerun and config_file args. |
| cie_thread_configurator/include/cie_thread_configurator/thread_configurator_node.hpp | Updates ctor signature + adds doc about param-based config loading. |
| cie_thread_configurator/include/cie_thread_configurator/prerun_node.hpp | Updates ctor signature to accept rclcpp::NodeOptions. |
| cie_thread_configurator/CMakeLists.txt | Builds/installs two executables and installs launch files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cie_thread_configurator/include/cie_thread_configurator/thread_configurator_node.hpp
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…_configurator_node.hpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Port the structural changes from autowarefoundation/agnocast#1234 to
cie_thread_configurator, moving from a bespoke CLI-based interface to a ROS-parameter / launch-file friendly layout.What changed
prerun_nodeis now its own executable. It used to be selected fromthread_configurator_nodevia--prerun. A dedicatedsrc/prerun_node_main.cppspins the node and callsdump_yaml_config(std::filesystem::current_path())on shutdown.PrerunNode's constructor now takesconst rclcpp::NodeOptions &, matching the upstream signature as a preparation step toward future composability (norclcpp_componentsregistration is added in this PR).thread_configurator_nodereads its YAML path from a ROS parameter.ThreadConfiguratorNodedeclares aconfig_fileparameter, loads the YAML, and performs hardware validation inside its constructor. On failure it throwsstd::runtime_error(missing parameter, bad file path, or hardware mismatch), whichmaincatches to exit with code 1 and a[ERROR] ...message. It also logsLoaded config from: <path>on success. LikePrerunNode, its constructor now acceptsrclcpp::NodeOptions, again as a structural preparation (no components registration added here).src/main.cppis renamed tosrc/thread_configurator_node_main.cpp(viagit mv) so the file name mirrorsprerun_node_main.cppand removes the ambiguous top-levelmain.cpp. CLI parsing,validate_hardware_info, andspin_prerun_nodeare removed; the existingspin_once()+all_applied()+apply_deadline_configs/exist_deadline_configinteractive flow is preserved unchanged.launch/thread_configurator.launch.xmlwith two args:prerun(bool, defaultfalse) andconfig_file(string, default"").if/unlessswitch betweenprerun_nodeandthread_configurator_node. Installed undershare/cie_thread_configurator/launch/via CMake.ros2 run cie_thread_configurator prerun_nodeandros2 run cie_thread_configurator thread_configurator_node --ros-args -p config_file:=..., with the launch-file alternatives added.Design decisions / scope
--prerun/--config-fileCLI flags — the old invocation just fails, no deprecation warning is emitted.Related links
How was this PR tested?
pre-commit run --all-files)colcon build)cie_sample_applicationNotes for reviewers