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

Fix empty sequence in moveit_setup_assistant #2406

Merged
merged 2 commits into from
Nov 10, 2020
Merged

Conversation

DLu
Copy link
Contributor

@DLu DLu commented Nov 5, 2020

Description

There's a bug right now in the generation of fake_controllers.yaml such that if no poses are found, it will output invalid yaml.

initial:  # Define initial robot poses.
#  - group: right_arm
#    pose: home

(and then the end of the file)

This results in a cryptic error like

load_parameters: unable to set parameters (last param was [/controller_list=[SNIP]]): cannot marshal None unless allow_none is enabled

With this PR, it now generates

initial:  # Define initial robot poses.
  []
#  - group: group
#    pose: home

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@DLu DLu requested a review from rhaschke as a code owner November 5, 2020 17:24
@davetcoleman
Copy link
Member

davetcoleman commented Nov 5, 2020

Not required for approval but @mohmadAyman adding this feature during his GSoC program 7097464

@gavanderhoorn
Copy link
Member

This is similar to #1680.

@codecov
Copy link

codecov bot commented Nov 5, 2020

Codecov Report

Merging #2406 (e22daa8) into master (382aa5a) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2406      +/-   ##
==========================================
- Coverage   60.22%   60.20%   -0.01%     
==========================================
  Files         352      352              
  Lines       26402    26402              
==========================================
- Hits        15898    15893       -5     
- Misses      10504    10509       +5     
Impacted Files Coverage Δ
...t_setup_assistant/src/tools/moveit_config_data.cpp 19.21% <ø> (ø)
moveit_core/robot_model/src/joint_model_group.cpp 62.97% <0.00%> (-2.46%) ⬇️
...e/src/parameterization/model_based_state_space.cpp 70.40% <0.00%> (-2.39%) ⬇️
...nning_scene_monitor/src/planning_scene_monitor.cpp 68.16% <0.00%> (-0.15%) ⬇️
...meterization/work_space/pose_model_state_space.cpp 85.30% <0.00%> (+0.74%) ⬆️
.../ompl_interface/src/detail/constrained_sampler.cpp 60.00% <0.00%> (+17.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 382aa5a...e22daa8. Read the comment docs.

@rhaschke
Copy link
Contributor

rhaschke commented Nov 5, 2020

Would be nice, if the empty list would follow the commented example, like this:

initial:  # Define initial robot poses.
#  - group: group
#    pose: home
  []

Copy link
Member

@tylerjw tylerjw left a comment

Choose a reason for hiding this comment

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

I tested this and it works as described.

Copy link
Contributor

@mayman99 mayman99 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing it!

@rhaschke rhaschke merged commit c616ef9 into moveit:master Nov 10, 2020
@tylerjw tylerjw mentioned this pull request Apr 9, 2021
tylerjw pushed a commit to tylerjw/moveit that referenced this pull request Apr 29, 2021
@tylerjw tylerjw mentioned this pull request Apr 29, 2021
tylerjw pushed a commit to tylerjw/moveit that referenced this pull request May 3, 2021
@DLu DLu deleted the yaml_problems branch June 2, 2021 14:15
sjahr pushed a commit to sjahr/moveit that referenced this pull request Jun 21, 2024
* (moveit_py) add trajectory execution manager

* (moveit_py) add __bool__ to ExecutionStatus

* (moveit_py) Update copyright header of changed files

* (moveit_py) add comment referencing issue

* Rename init_trajectory_execution_manager -> initTrajectoryExecutionManager

* (moveit_py) python functions snake_case

* (moveit_py) fix styling

---------
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.

6 participants