Allow specification of controller names for trajectories #1832
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for helping in improving MoveIt and open source robotics! |
rhaschke
reviewed
Jan 14, 2020
moveit_ros/planning/plan_execution/include/moveit/plan_execution/plan_representation.h
Outdated
Show resolved
Hide resolved
moveit_plugins/moveit_fake_controller_manager/src/moveit_fake_controller_manager.cpp
Show resolved
Hide resolved
rhaschke
approved these changes
Jan 15, 2020
| @@ -262,12 +268,9 @@ class MoveItFakeControllerManager : public moveit_controller_manager::MoveItCont | |||
| * Controllers are all active and default. | |||
| */ | |||
| moveit_controller_manager::MoveItControllerManager::ControllerState | |||
| getControllerState(const std::string& /*name*/) override | |||
There was a problem hiding this comment.
Shouldn't the same be implemented for the SimpleControllerManager?
There was a problem hiding this comment.
Probably. @llach, could you provide the same implementation for SimpleControllerManager as well?
I suggest to split this PR into two separate commits then:
- forwarding the controller set
- fixing FakeControllerManager and SimpleControllerManager to consider the config
default
So, Luca, please cleanup the commit history by appropriately squashing your changes.
|
@llach, could you please squash my fixup, so that we can rebase-merge this PR? Thanks. |
|
Done! |
Codecov Report
@@ Coverage Diff @@
## master #1832 +/- ##
==========================================
- Coverage 48.5% 48.47% -0.04%
==========================================
Files 298 298
Lines 23302 23307 +5
==========================================
- Hits 11302 11297 -5
- Misses 12000 12010 +10
Continue to review full report at Codecov.
|
- ExecutableTrajectory has a new member controller_names_ - PlanExecution includes that list when pushing trajctory parts to the TrajectoryExecutionManager
When reading controller configurations, "default" is ignored as
opposed to what is currently written in the documentation.
MoveIt{Fake|Simple}ControllerManager now maintains map of controller name and
ControllerState where "default" is initialized as "false" or whatever
is given in the configuration.
MoveIt{Fake|Simple}ControllerManager::getControllerState() now returns the
state that is held in that map instead of a default one.
|
Congrats on getting your first MoveIt pull request merged and improving open source robotics! |
2 tasks
2 tasks
sjahr
pushed a commit
to sjahr/moveit
that referenced
this pull request
Jun 21, 2024
…e Parameterization (moveit#1832) * add velocity and accelerations scaling when using custom limits for time parameterization * add scaling when passing in vecotor of joint-limits * add function descriptions * add verifyScalingFactor helper function * make map const * address feedback * add comment Co-authored-by: Michael Wiznitzer <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds a list of controller names to
ExecutableTrajectorywhich is passed to theTrajectoryExecutionManager(TEM) inPlanExecution::executeAndMonitor(). This allows users to specify which controllers should execute a certain trajectory (or only a part of it). The TEM already had the ability to manage different controllers for different trajectory parts, it just was not used before.Additonally, the
FakeControllerManagernow reads thedefaultparameter from thecontroller_liston the param server and initializes theControllerStateaccordingly. Before, eachControllerStatehaddefault_andactive_set totruewhich lead the TEM's controller selection mechanism to choose any controller when no explicit controller name was given.The documentation mentions this parameter, but neither SimpleControllerManager nor FakeControllerManager use it.
When I did these changes, I stumbled upon comments hinting that controller switching was possible or discontinued. Someone with a better overview of MoveIt should probably decide whether it okay to allow it again.
Checklist