Skip to content

refactor(cie): restructure thread_configurator into ROS-param-based nodes#46

Open
atsushi421 wants to merge 4 commits intomainfrom
refactor/cie-thread-configurator-ros-params
Open

refactor(cie): restructure thread_configurator into ROS-param-based nodes#46
atsushi421 wants to merge 4 commits intomainfrom
refactor/cie-thread-configurator-ros-params

Conversation

@atsushi421
Copy link
Copy Markdown
Collaborator

@atsushi421 atsushi421 commented Apr 10, 2026

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_node is now its own executable. It used to be selected from thread_configurator_node via --prerun. A dedicated src/prerun_node_main.cpp spins the node and calls dump_yaml_config(std::filesystem::current_path()) on shutdown. PrerunNode's constructor now takes const rclcpp::NodeOptions &, matching the upstream signature as a preparation step toward future composability (no rclcpp_components registration is added in this PR).
  • thread_configurator_node reads its YAML path from a ROS parameter. ThreadConfiguratorNode declares a config_file parameter, loads the YAML, and performs hardware validation inside its constructor. On failure it throws std::runtime_error (missing parameter, bad file path, or hardware mismatch), which main catches to exit with code 1 and a [ERROR] ... message. It also logs Loaded config from: <path> on success. Like PrerunNode, its constructor now accepts rclcpp::NodeOptions, again as a structural preparation (no components registration added here).
  • src/main.cpp is renamed to src/thread_configurator_node_main.cpp (via git mv) so the file name mirrors prerun_node_main.cpp and removes the ambiguous top-level main.cpp. CLI parsing, validate_hardware_info, and spin_prerun_node are removed; the existing spin_once() + all_applied() + apply_deadline_configs / exist_deadline_config interactive flow is preserved unchanged.
  • New canonical launch file launch/thread_configurator.launch.xml with two args: prerun (bool, default false) and config_file (string, default ""). if/unless switch between prerun_node and thread_configurator_node. Installed under share/cie_thread_configurator/launch/ via CMake.
  • README usage examples updated to ros2 run cie_thread_configurator prerun_node and ros2 run cie_thread_configurator thread_configurator_node --ros-args -p config_file:=..., with the launch-file alternatives added.

Design decisions / scope

  • No backward compatibility for the removed --prerun / --config-file CLI flags — the old invocation just fails, no deprecation warning is emitted.

Related links

How was this PR tested?

  • Format (pre-commit run --all-files)
  • Build (colcon build)
  • Test with cie_sample_application

Notes for reviewers

…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>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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_node into its own executable (prerun_node_main.cpp) and executable target.
  • Update thread_configurator_node to load YAML from a config_file ROS parameter and perform hardware validation in-node.
  • Add an XML launch file to switch between prerun_node and thread_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.

atsushi421 and others added 2 commits April 11, 2026 08:04
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…_configurator_node.hpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@atsushi421 atsushi421 marked this pull request as ready for review April 10, 2026 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants