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

MoveItConfigBuilder support for multiple instaces #1807

Merged

Conversation

MarcoMagriDev
Copy link
Contributor

Creating two instances of MoveItConfigBuilder seems to be unsupported due to conflicting MoveItConfigs.
This is an example launcher to reproduce the issue test_builder.zip.
As you can notice by running the script (with the current version of the MoveItConfigBuilder) the attributes of two different MoveItConfigs (produced by two different MoveItConfigBuilder) share the same id and values (the one of the fanuc in the provided example).

The proposed solution simply consists in moving the instatiation of the MoveItConfigs inside the __init__() method of MoveItConfigBuilder. By doing so the problem seems to be solved (running again the script after applying the modification result in the expected behavior).

Copy link
Member

@JafarAbdi JafarAbdi left a comment

Choose a reason for hiding this comment

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

Thanks!

@henningkayser henningkayser added the backport-humble Mergify label that triggers a PR backport to Humble label Dec 14, 2022
@codecov
Copy link

codecov bot commented Dec 14, 2022

Codecov Report

Base: 50.27% // Head: 50.27% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (caf12b1) compared to base (e60bced).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1807      +/-   ##
==========================================
+ Coverage   50.27%   50.27%   +0.01%     
==========================================
  Files         374      374              
  Lines       31286    31286              
==========================================
+ Hits        15726    15727       +1     
+ Misses      15560    15559       -1     
Impacted Files Coverage Δ
...dl_kinematics_plugin/src/kdl_kinematics_plugin.cpp 78.79% <0.00%> (-1.13%) ⬇️
...nning_scene_monitor/src/planning_scene_monitor.cpp 45.69% <0.00%> (+0.44%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@JafarAbdi JafarAbdi merged commit 25d086c into moveit:main Dec 14, 2022
mergify bot pushed a commit that referenced this pull request Dec 14, 2022
JafarAbdi pushed a commit that referenced this pull request Dec 14, 2022
(cherry picked from commit 25d086c)

Co-authored-by: Marco Magri <94347649+MarcoMagriDev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble Mergify label that triggers a PR backport to Humble
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants