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

Improve evaluate_paramenter_dict exceptions error message #320

Merged
merged 1 commit into from
Dec 8, 2022

Conversation

FelipeGdM
Copy link
Contributor

@FelipeGdM FelipeGdM commented Jun 27, 2022

This PR updates the evaluate_paramenter_dict() possible exceptions adding the name of the parameter that caused the exception, thus helping find the root of the problem when it occurs

For example, if a NoneType argument is passed to the function, the thrown message would be

TypeError: Allowed value types are bytes, bool, int, float, str, Sequence[bool], Sequence[int], 
Sequence[float], Sequence[str]. Got <class 'NoneType'>. If the parameter is meant to be 
a string, try wrapping it in launch_ros.parameter_descriptions.ParameterValue(value, value_type=str)

Within a large project, with dozens of launch arguments, it is not trivial to discover which argument received the invalid value. As the name of the argument being evaluated is already available in the scope of the exception, adding it to the error message would be really helpful when dealing with this problem.

As discussed in some ROS 2 tickets (#291, ros2/launch#412), one of the main issues of the launch system is the lack of helpful information when something goes wrong, this PR is a small step in the direction of solving this issue

Signed-off-by: Felipe Gomes de Melo <felipegmelo@usp.br>
@methylDragon
Copy link
Contributor

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

@methylDragon methylDragon merged commit d38055c into ros2:rolling Dec 8, 2022
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.

None yet

2 participants