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

"yes" and "no" string parameters are confused for bool parameters #1983

Closed
Alberto-Neri opened this issue Aug 9, 2022 · 4 comments
Closed
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@Alberto-Neri
Copy link

  • Operating System:
    Windows 10
  • Installation type:
    ROS2 Foxy binaries
  • DDS implementation:
    Cyclone-DDS
  • Client library (if applicable):
    rclcpp

Steps to reproduce issue

Use whatever node you want and add somewhere:

std::string calib;
try {
    this->declare_parameter<std::string>("calib", "no");
    this->get_parameter_or("calib", calib, std::string("no"));
}
catch (std::exception e) {
    std::cout << e.what() << "\n";
}
std::cout << calib << "\n";

Then try run the node with:

ros2 run pkg_name exe_name --ros-args -p "calib:=yes"

Expected behavior

The CLI prints yes.

Actual behavior

Exception triggered with description:
parameter 'calib' has invalid type: expected [string] got [bool].

Additional information

If I launch the node with:
ros2 run pkg_name exe_name --ros-args -p "calib:=ok"
I get the expected behaviour (ok printed).

If I modify the code to accept a bool parameter, like this:

bool calib;
try {
    this->declare_parameter<bool>("calib", false);
    this->get_parameter_or("calib", calib, false);
}
catch (std::exception e) {
    std::cout << e.what() << "\n";
} 
std::cout << calib << "\n";

and do: ros2 run pkg_name exe_name --ros-args -p "calib:=yes"
I get no exception and it prints 1, that is the same as true!!
While doing: ros2 run pkg_name exe_name --ros-args -p "calib:=ok"
gives me the inverse exception as before:
parameter 'calib' has invalid type: expected [bool] got [string]. !!!

Is this expected?

@clalancette
Copy link
Contributor

Is this expected?

Unfortunately, yes, though it is a bug. We interpret a bunch of this stuff as YAML, and since "yes"/"no" gets automatically converted to a boolean in YAML, that's how it ends up here.

We partially fixed this in ros2/ros2cli#684 , where you can now force the type to be used in ros2 param by using the YAML !!str, but we didn't fix it in rclcpp or elsewhere.

For now, I'd say the best workaround is to avoid yes and no (and true and false for that matter) in your strings. It's not ideal, but until we have full support for the !!str syntax, it is the best we can do.

@clalancette clalancette added bug Something isn't working help wanted Extra attention is needed labels Aug 9, 2022
@iuhilnehc-ynos
Copy link
Collaborator

and since "yes"/"no" gets automatically converted to a boolean in YAML, that's how it ends up here.

I am afraid it's not correct. The "yes"/"no" or 'yes'/'no' can be parsed as a string.
Adding '!!str' seems good to me.

@Barry-Xu-2018
Copy link
Collaborator

I am afraid it's not correct. The "yes"/"no" or 'yes'/'no' can be parsed as a string.

Yes.
Using " or ' can set calib correctly.

ros2 run pkg_name exe_name --ros-args -p 'calib:="yes"'   

Using !!str is another way (ros2/rcl#999)

ros2 run pkg_name exe_name --ros-args -p 'calib:=!!str yes' 

@fujitatomoya
Copy link
Collaborator

@Alberto-Neri direct solution to this issue would be just adding quote mentioned in #1983 (comment), we have also added yaml string tag in ros2/rcl#999.

I will go ahead to close this issue, if you have any more questions or issues, feel free to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants