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

bugs in load_parameter_file #960

Open
huweiATgithub opened this issue Jan 12, 2025 · 3 comments
Open

bugs in load_parameter_file #960

huweiATgithub opened this issue Jan 12, 2025 · 3 comments
Labels
more-information-needed Further information is required

Comments

@huweiATgithub
Copy link

Bug report

Required Info:

  • Version or commit hash:
    def get_parameter_value(*, string_value):
    """Guess the desired type of the parameter based on the string value."""
    value = ParameterValue()
    try:
    yaml_value = yaml.safe_load(string_value)
    except yaml.parser.ParserError:
    yaml_value = string_value

Steps to reproduce issue

The current implementation has an extra yaml.safe_load for parameter value which has been loaded once already.
This will result in wrong results, e.g.,

key: ''

is once being loaded to {"key": ""}. Another load for the empty string "" will convert it into None.

Implementation considerations

Consider removing the extra yaml load.

@fujitatomoya
Copy link
Collaborator

@huweiATgithub can you provide the complete procedure (command line steps) that explain the problem here?

@huweiATgithub
Copy link
Author

@huweiATgithub can you provide the complete procedure (command line steps) that explain the problem here?

  1. having a node with a string type parameter
  2. load parameter from a file using ros2 param load with file:
    /**:
      ros__parameters:
        param_name: ''
  3. Notice an exception.

fujitatomoya added a commit to fujitatomoya/ros2_test_prover that referenced this issue Jan 13, 2025
@fujitatomoya
Copy link
Collaborator

using fujitatomoya/ros2_test_prover@1fcc859 and the following procedure that you provided above, it does not generate the exception.

### start the node with string parameter
root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 run prover_rclpy ros2cli_960 
[INFO] [1736797972.379024312] [minimal_param_node]: Got my_parameter: 
[INFO] [1736798063.849885193] [minimal_param_node]: Got my_parameter: 

### set the string parameter with ros2 param load
root@tomoyafujita:~/ros2_ws/colcon_ws# cat param_string_empty.yaml 
/**:
  ros__parameters:
    my_parameter: ''

root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 param load minimal_param_node param_string_empty.yaml 
Set parameter my_parameter successful
root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 param get /minimal_param_node my_parameter
String value is: 
root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 param describe /minimal_param_node my_parameter
Parameter name: my_parameter
  Type: string
  Constraints:
root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 param load minimal_param_node param_string_empty.yaml 
Set parameter my_parameter successful
root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 param get /minimal_param_node my_parameter
String value is: 
root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 param describe /minimal_param_node my_parameter
Parameter name: my_parameter
  Type: string
  Constraints:

i would like to ask you again, could you please provide the Short, Self Contained, Correct (Compilable), Example?

@fujitatomoya fujitatomoya added the more-information-needed Further information is required label Jan 13, 2025
fujitatomoya added a commit to fujitatomoya/ros2_test_prover that referenced this issue Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more-information-needed Further information is required
Projects
None yet
Development

No branches or pull requests

2 participants