More Helpful Error Messages (backport #275) #289
Conversation
![mergify[bot]](https://avatars.githubusercontent.com/in/10562?s=60&v=4){kind=small}
* More Helpful Error Messages Signed-off-by: David V. Lu <davidvlu@gmail.com> * Linting Signed-off-by: David V. Lu <davidvlu@gmail.com> (cherry picked from commit b6f187a)
|
CI with foxy repos.url:
|
|
Windows error seems to be flaky and unrelated. CI re-run : Attempt 2 : |
|
@adityapande-1995 Anything I should do here? |
|
@jacobperron do you think this windows rosbag error is related to the shutdown issue : ros2/rosbag2#926 ? In this case the CI is not able to find the |
|
The failure looks unrelated to me. You can trigger a vanilla foxy build to be sure, but I would be okay merging this. |
|
It would be nice to create a ticket in rosbag2 for this unrelated failure. |
|
Vanilla foxy CI for windows : Same error appears in the vanialla builld. I'll merge this and ticket the issue. |
| @@ -69,9 +69,17 @@ def check_sequence_type_is_allowed(sequence): | |||
| if isinstance(value[0], Substitution): | |||
| # Value is a list of substitutions, so perform them to make a string | |||
| evaluated_value = perform_substitutions(context, list(value)) | |||
| yaml_evaluated_value = yaml.safe_load(evaluated_value) | |||
| if yaml_evaluated_value is None: | |||
| yaml_evaluated_value = '' | |||
There was a problem hiding this comment.
Removing this check breaks existing launch files in Foxy (and Galactic). For example, the following used to work:
import launch
from launch.actions import DeclareLaunchArgument
from launch.substitutions import LaunchConfiguration
from launch_ros.actions import Node
def generate_launch_description():
return launch.LaunchDescription([
DeclareLaunchArgument('foo', default_value=''),
Node(
package='demo_nodes_py',
executable='talker',
name='talker_node',
output='screen',
parameters = [
{'foo': LaunchConfiguration('foo')}
]
),
])because the LaunchConfiguration substitution evaluated to None which used to translate to the empty string.
I think we can easily fix the issue in Foxy and Galactic by adding back L73-74 above, but I'm not sure what the correct behavior should be on Rolling.
@adityapande-1995 @DLu thoughts?
cc/ @Yadunund
There was a problem hiding this comment.
See #300 for a proposed fix.
This is an automatic backport of pull request #275 done by Mergify.
Mergify commands and options
More conditions and actions can be found in the documentation.
You can also trigger Mergify actions by commenting on this pull request:
@Mergifyio refreshwill re-evaluate the rules@Mergifyio rebasewill rebase this PR on its base branch@Mergifyio updatewill merge the base branch into this PR@Mergifyio backport <destination>will backport this PR on<destination>branchAdditionally, on Mergify dashboard you can:
Finally, you can contact us on https://mergify.com