Skip to content

[Jazzy] Improve the compatibility of processing YAML parameter files#1549

Merged
ahcorde merged 6 commits into
ros2:jazzyfrom
Barry-Xu-2018:review/jazzy/topic-update-parameter_dict_from_yaml_file
Dec 23, 2025
Merged

[Jazzy] Improve the compatibility of processing YAML parameter files#1549
ahcorde merged 6 commits into
ros2:jazzyfrom
Barry-Xu-2018:review/jazzy/topic-update-parameter_dict_from_yaml_file

Conversation

@Barry-Xu-2018
Copy link
Copy Markdown
Contributor

Description

The implementation of the parameter_dict_from_yaml_file() function differs significantly between the rolling version and the Jazzy version, so I made separate modifications for the Jazzy version.

Fixes #1546

And also supports the following YAML parameter format

/**/node_name:
   ros__parameters:
      ...
/*:
   node_name:
       ros__parameters:

Is this user-facing behavior change?

Yes. parameter_dict_from_yaml_file() supports a more complete parameter file format (consistent with rcl_yaml_param_parser).

Did you use Generative AI?

No

Additional Information

No

Signed-off-by: Barry Xu <barry.xu@sony.com>
@Barry-Xu-2018 Barry-Xu-2018 changed the title Improve the compatibility of processing YAML parameter files [Jazzy] Improve the compatibility of processing YAML parameter files Dec 2, 2025
@Barry-Xu-2018 Barry-Xu-2018 marked this pull request as draft December 3, 2025 09:22
@Barry-Xu-2018
Copy link
Copy Markdown
Contributor Author

I found that the handling of cases where /**/node_name is included in the parameter file is incomplete. The ** represents one or multiple levels of namespaces. Currently, the code only handles cases with a single-level namespace.
I will fix this and then reopen to review.

…'/**/node_name'

Signed-off-by: Barry Xu <barry.xu@sony.com>
@Barry-Xu-2018 Barry-Xu-2018 marked this pull request as ready for review December 4, 2025 02:40
Copilot AI review requested due to automatic review settings December 4, 2025 02:40
Copy link
Copy Markdown

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

This PR enhances the parameter_dict_from_yaml_file() function in the Jazzy version to support additional YAML parameter file formats, aligning it with the functionality of rcl_yaml_param_parser. The changes enable processing of wildcard namespace patterns (/* and /**/node_name) and nested namespace/node definitions.

Key changes:

  • Extended wildcard matching to support /* and /**/node_name patterns in addition to the existing /** pattern
  • Added support for nested YAML structures where namespaces contain node definitions (e.g., /namespace: node_name: ros__parameters:)
  • Implemented logic to handle both absolute and relative node name formats in parameter files

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
rclpy/rclpy/parameter.py Rewrote parameter file parsing logic to handle multiple YAML formats including wildcard patterns and nested namespace/node structures
rclpy/test/test_parameter.py Added comprehensive test coverage for new YAML parameter formats including wildcard matching and various namespace/node combinations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rclpy/rclpy/parameter.py
Comment thread rclpy/rclpy/parameter.py Outdated
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Barry Xu <barry.xu@sony.com>
Copy link
Copy Markdown
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm with green CI.

@fujitatomoya
Copy link
Copy Markdown
Collaborator

@Barry-Xu-2018 if i understand correctly, we do not need #1552 to jazzy and humble right?

@fujitatomoya
Copy link
Copy Markdown
Collaborator

Pulls: #1549
Gist: https://gist.githubusercontent.com/fujitatomoya/e7fdcb83d91f87b05e336439e95d02f7/raw/068f73bc82ddf15afe433309a4d607ef3f69c5aa/ros2.repos
BUILD args: --packages-above-and-dependencies rclpy
TEST args: --packages-above rclpy
ROS Distro: jazzy
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/17783

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@Barry-Xu-2018
Copy link
Copy Markdown
Contributor Author

@fujitatomoya

@Barry-Xu-2018 if i understand correctly, we do not need #1552 to jazzy and humble right?

Yes. You are right.

@ahcorde ahcorde merged commit 6b3a5c3 into ros2:jazzy Dec 23, 2025
3 checks passed
@fujitatomoya
Copy link
Copy Markdown
Collaborator

@Mergifyio backport humble

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Dec 23, 2025

backport humble

✅ Backports have been created

Details

mergify Bot pushed a commit that referenced this pull request Dec 23, 2025
…1549)

Signed-off-by: Barry Xu <barry.xu@sony.com>
(cherry picked from commit 6b3a5c3)

# Conflicts:
#	rclpy/rclpy/parameter.py
#	rclpy/test/test_parameter.py
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.

4 participants